feat:[sal] improve the error return of sal #10719
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves error handling and validation in the SAL (Socket Abstraction Layer) component by enhancing parameter checking and making configuration more flexible.
- Enhanced socket initialization with comprehensive parameter validation including protocol range checks and domain/type/protocol combination validation
- Made SOCKET_TABLE_STEP_LEN configurable through Kconfig instead of hardcoded define
- Improved error return values to provide more specific error information instead of generic -1
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| components/net/sal/src/sal_socket.c | Enhanced socket_init() with parameter validation, improved error logging, and better error return handling |
| components/net/sal/include/sal_socket.h | Added new includes and macros for protocol validation and domain/type/protocol combination checking |
| components/net/sal/Kconfig | Added configurable SOCKET_TABLE_STEP_LEN parameter |
Comments suppressed due to low confidence (1)
components/net/sal/src/sal_socket.c:1
- English: The constants PROTOCOL_TLS and PROTOCOL_DTLS are referenced but may not be defined. This could cause compilation errors if TLS support is not enabled or these constants are not available in the included headers.
中文：引用了 PROTOCOL_TLS 和 PROTOCOL_DTLS 常量，但这些常量可能未定义。如果未启用 TLS 支持或这些常量在包含的头文件中不可用，这可能导致编译错误。
/*
| #include "sal_low_lvl.h" | ||
| #include "sal_msg.h" | ||
| #include "sal_netdb.h" | ||
| #include "sal_tls.h" |
There was a problem hiding this comment.
English: These includes appear to be added unnecessarily. The header file should only include what it directly uses. If these headers are needed for the VALID_COMBO macro, consider moving the macro to the source file or including only the specific headers that define the required constants.
中文：这些包含文件似乎是不必要添加的。头文件应该只包含它直接使用的内容。如果这些头文件是 VALID_COMBO 宏所需要的，考虑将宏移动到源文件中，或者只包含定义所需常量的特定头文件。
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@Rbb666 这个是不是可以考虑放到sal_socket.h中，后面用户直接调用这个sal_socket.h就可以了
There was a problem hiding this comment.
不推荐加到这里，交给用户加吧，其他的文件也是这么干的：
| ((domain) == AF_NETLINK && (type) == SOCK_RAW && (protocol) == 0) || \ | ||
| /* Add more combos for AF_CAN, AF_AT, AF_WIZ if needed */ \ | ||
| 1 /* Default allow for unknown, but log warning */ \ |
There was a problem hiding this comment.
English: The VALID_COMBO macro always returns true due to the final '1' condition, making the validation ineffective. This defeats the purpose of parameter validation. Consider removing this fallback or implementing proper validation for all supported domain/type/protocol combinations.
中文：由于最后的 '1' 条件，VALID_COMBO 宏总是返回 true，使验证失效。这违背了参数验证的目的。考虑移除这个回退条件，或者为所有支持的域/类型/协议组合实现适当的验证。
| ((domain) == AF_NETLINK && (type) == SOCK_RAW && (protocol) == 0) || \ | |
| /* Add more combos for AF_CAN, AF_AT, AF_WIZ if needed */ \ | |
| 1 /* Default allow for unknown, but log warning */ \ | |
| ((domain) == AF_NETLINK && (type) == SOCK_RAW && (protocol) == 0) \ | |
| /* Add more combos for AF_CAN, AF_AT, AF_WIZ if needed */ \ |
Copilot uses AI. Check for mistakes.
components/net/sal/Kconfig
Outdated
| The ability that check internet status is provided by RT-Thread. | ||
|
|
||
| config SOCKET_TABLE_STEP_LEN | ||
| int "Configurate socket table step length" |
There was a problem hiding this comment.
English: 'Configurate' should be 'Configure' in the description text.
中文：描述文本中的 'Configurate' 应该是 'Configure'。
| int "Configurate socket table step length" | |
| int "Configure socket table step length" |
Copilot uses AI. Check for mistakes.
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-09-24 20:06 CST)
📝 Review Instructions
|
|
|
||
| #define VALID_PROTOCOL(protocol) ((protocol) >= 0 && (protocol) <= IPPROTO_RAW) | ||
| #define VALID_COMBO(domain, type, protocol) \ | ||
| ( \ |
| #include "sal_low_lvl.h" | ||
| #include "sal_msg.h" | ||
| #include "sal_netdb.h" | ||
| #include "sal_tls.h" |
There was a problem hiding this comment.
不推荐加到这里，交给用户加吧，其他的文件也是这么干的：
拉取/合并请求描述：(PR description)
[
1.完善sal初始化参数检查
2.修改
SOCKET_TABLE_STEP_LEN为用户可配置参数为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up