-
Notifications
You must be signed in to change notification settings - Fork 38
Add callback exposing client source port #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extends the RELP library to provide client source port information to callback functions. It adds a new relpEngineSetSyslogRcv3 API that includes the client's port number along with hostname and IP address.
- Add
pRemHostPortfield to TCP structure to track client source port - Implement new
relpEngineSetSyslogRcv3callback API with port parameter - Update internal logic to extract and store port information from socket addresses
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tcp.h | Add pRemHostPort field to store client source port |
| src/tcp.c | Extract port from socket address and manage memory allocation |
| src/scsyslog.c | Invoke new callback with port parameter when available |
| src/relp.h | Add onSyslogRcv3 callback function pointer to engine structure |
| src/relp.c | Implement new API function and dummy callback |
| src/librelp.h | Export new relpEngineSetSyslogRcv3 API function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
9de3b00 to
7097c7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a new callback to expose the client's source port, which is a useful enhancement. The implementation looks mostly correct.
My main concern is with the error handling in src/tcp.c where the getnameinfo() call is updated. The new error handling is less robust than the previous version and could lead to issues if getnameinfo() fails. I've left a specific comment with a suggestion to fix this.
Additionally, there are several formatting changes throughout the pull request, mainly replacing tabs with spaces for indentation. This makes the diff harder to read and introduces style inconsistencies. It would be great if these formatting changes could be reverted to keep the focus on the logical changes and maintain a consistent coding style across the project.
7097c7d to
84b29ed
Compare
This is handled via a new "on connection callback"
84b29ed to
9a7d8bf
Compare
Summary
relpEngineSetSyslogRcv3API that passes hostname, IP, and portTesting
make -j$(nproc)make checkhttps://chatgpt.com/codex/tasks/task_e_68bbf884f07483328ee2e5a0af8b9c1a