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 宏所需要的，考虑将宏移动到源文件中，或者只包含定义所需常量的特定头文件。
There was a problem hiding this comment.
@Rbb666 这个是不是可以考虑放到sal_socket.h中，后面用户直接调用这个sal_socket.h就可以了
There was a problem hiding this comment.
不推荐加到这里，交给用户加吧，其他的文件也是这么干的：
There was a problem hiding this comment.
因为考虑说加上对socket参数的检查，所以这里才会包含头文件的，所以看看这里会不会有其他影响？
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-09-30 11:18 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整合成两个提交记录，把最后一笔提交覆盖掉第一笔 |
3 participants
拉取/合并请求描述：(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