Skip to content

socket: implement accept() with syscalls_sys_accept4#469

Open
xvuko wants to merge 2 commits into
masterfrom
xvuko/unacceptable
Open

socket: implement accept() with syscalls_sys_accept4#469
xvuko wants to merge 2 commits into
masterfrom
xvuko/unacceptable

Conversation

@xvuko

@xvuko xvuko commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

This will allow to deprecate/remove accept syscall.

TASK: RTOS-1289

Description

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (list targets here).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

  • This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  • I will merge this PR by myself when appropriate.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Apr 16, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

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 refactors the accept function to be an inline wrapper around accept4 with a flags value of 0. Feedback suggests that implementing accept as an inline function in the header is inconsistent with other socket wrappers in the library and may introduce portability issues. It is recommended to move the implementation to the source file as a regular function.

Comment thread include/sys/socket.h
Comment thread sys/socket.c Outdated
@github-actions

github-actions Bot commented Apr 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Unit Test Results

10 585 tests  ±0   9 915 ✅ ±0   52m 47s ⏱️ + 1m 14s
   649 suites ±0     670 💤 ±0 
     1 files   ±0       0 ❌ ±0 

Results for commit f972961. ± Comparison against base commit 41c649e.

♻️ This comment has been updated with latest results.

@xvuko xvuko force-pushed the xvuko/unacceptable branch from 86f6550 to b590d94 Compare April 16, 2026 17:12
@xvuko xvuko marked this pull request as ready for review April 16, 2026 17:36
@xvuko xvuko requested a review from Darchiv May 13, 2026 08:20
julianuziemblo
julianuziemblo reviewed May 25, 2026
View reviewed changes

@julianuziemblo julianuziemblo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while at it, we could potentially just remove syscalls_sys_accept, as we're still before release. @Darchiv what do you think?

Comment thread include/sys/socket.h Outdated
xvuko added 2 commits May 29, 2026 15:53
@xvuko
use non-static inlines for send() and receive()
c529b5a 
TASK: RTOS-1289
@xvuko
socket: implement accept() with syscalls_sys_accept4
f972961 
This will allow to deprecate/remove `accept` syscall.

TASK: RTOS-1289
@xvuko xvuko force-pushed the xvuko/unacceptable branch from b590d94 to f972961 Compare May 29, 2026 13:54
ziemleszcz
ziemleszcz reviewed Jun 9, 2026
View reviewed changes
Comment thread include/sys/socket.h
int socketpair(int domain, int type, int protocol, int socket_vector[2]);


inline int accept(int socket, struct sockaddr *address, socklen_t *address_len)

@ziemleszcz ziemleszcz Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With static inline, the extern inline declarations in sys/socket.c are probably no longer necessary.

@Darchiv

Darchiv commented Jun 16, 2026

Copy link
Copy Markdown
Member

while at it, we could potentially just remove syscalls_sys_accept, as we're still before release. @Darchiv what do you think?

Seems sane. Remove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ziemleszcz ziemleszcz ziemleszcz left review comments

@julianuziemblo julianuziemblo julianuziemblo left review comments

@Darchiv Darchiv Awaiting requested review from Darchiv

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xvuko @Darchiv @ziemleszcz @julianuziemblo