Skip to content
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

samples: sockets: socketpair: sample application and docs #25528

Merged
merged 1 commit into from
May 27, 2020
Merged

samples: sockets: socketpair: sample application and docs #25528

merged 1 commit into from
May 27, 2020

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented May 21, 2020

Support for the socketpair(2) system call was recently added for 2.3.0.

This change adds a sample application that demonstrates how it can be used.

Fixes #25527

@cfriedt cfriedt marked this pull request as draft May 21, 2020 15:31
@zephyrbot
Copy link
Collaborator

zephyrbot commented May 21, 2020

All checks are passing now.

checkpatch (informational only, not a failure)

-:333: WARNING:EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to using 'main', this function's name, in a string
#333: FILE: samples/net/sockets/socketpair/src/socketpair_example.c:156:
+				printf("main: closed fd %d\n",

- total: 0 errors, 1 warnings, 311 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPDX_LICENSE_TAG SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@pfalcon pfalcon added the area: Sockets Networking sockets label May 22, 2020
@cfriedt
Copy link
Member Author

cfriedt commented May 22, 2020

Hehe... so looks like I'm implementing nanosleep(2). Probably better to do that in a separate PR ;-)

@pfalcon saw #24730

Looks like newlib doesn't have its own implementation and just makes a syscall. Doesn't look like Zephyr has such a syscall atm. I was thinking of kludging it together with k_sleep(), k_msleep(), and k_cycle_get().

@cfriedt
Copy link
Member Author

cfriedt commented May 24, 2020

I'd like to leave this as a draft until #25559 and #25572 are merged.

@pfalcon
Copy link
Contributor

pfalcon commented May 25, 2020

I'd like to leave this as a draft until #25559 and #25572 are merged.

To set expectations right, this may be as well after the 2.3 release (and surely, not e who decides).

@cfriedt
Copy link
Member Author

cfriedt commented May 25, 2020

I'd like to leave this as a draft until #25559 and #25572 are merged.

To set expectations right, this may be as well after the 2.3 release (and surely, not e who decides).
Of course ;-)

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

@pfalcon
Copy link
Contributor

pfalcon commented May 26, 2020

From #25528 (comment)

@pfalcon saw #24730

And I didn't saw this and following lines. Because they were edited into an already posted comment, and the original content of that comment is what I got in email and considered it read. I mentioned this issue lately elsewhere. Please avoid editing comments for adding new information, post it as a new comment instead. It's ok to post a bunch of comments in row (if it's needed). (And I for one edit my comments, but only to fix typos/ambiguous wording).

@cfriedt
Copy link
Member Author

cfriedt commented May 26, 2020

It looks like there was a copy-paste error made several years ago that is breaking this PR.

c8aa657

typedef struct pthread_attr_t {
...
} pthread_attr_t;

@pfalcon
Copy link
Contributor

pfalcon commented May 26, 2020

@cfriedt: I don't see a copy-paste error there. It's quite a natural way to use the same name for both struct and typedef, but if you hint that it plays bad with #define which is supposed to target just one of those names, then yeah, could be.

@cfriedt
Copy link
Member Author

cfriedt commented May 26, 2020

@pfalcon - it's inconsistence. Elsewhere in the file there is e.g.

typedef struct pthread_mutex {
...
} pthread_mutex_t;

The actual error here is specific to nrf52bsim I think. I didn't see the entirety of the error message... but posix types are defined twice.

-include /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/arch/posix/include/posix_cheats.h

Support for the socketpair(2) system call was recently
added for 2.3.0 .

This change adds a sample application that demonstrates
how it can be used.

Fixes #25527

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
@pfalcon
Copy link
Contributor

pfalcon commented May 26, 2020

nrf52bsim is another instance of CONFIG_ARCH_POSIX, aka native_posix issue-alike.

Anyway, feel free to propose a fix. As usual, submitting it as a separate PR raises chances for it to be processed sooner rather than later (but I assume, @carlescufi is busy with high-priority fixes by now).

@cfriedt cfriedt marked this pull request as ready for review May 26, 2020 10:37
@cfriedt
Copy link
Member Author

cfriedt commented May 26, 2020

@carlescufi - sample application and docs would be good to accompany socketpair for 2.3.0. I'm sure it's too late now, but just thought I would throw it out there.

@pfalcon pfalcon added the area: POSIX POSIX API Library label May 26, 2020
@pfalcon
Copy link
Contributor

pfalcon commented May 26, 2020

@carlescufi - sample application and docs would be good to accompany socketpair for 2.3.0. I'm sure it's too late now, but just thought I would throw it out there.

Given that socketpair() is just introduced for 2.3, we hardly can make it worse for anyone with further changes, only better. @carlescufi, adding 2.3 milestone to help you not forget about this case ;-).

@pfalcon pfalcon added this to the v2.3.0 milestone May 26, 2020
@carlescufi
Copy link
Member

I am merging this because it's a sample that demonstrates a new feature that was introduced in 2.3.0.

@carlescufi carlescufi merged commit 2710c18 into zephyrproject-rtos:master May 27, 2020
@cfriedt cfriedt deleted the issue/25527/sample-and-writeup-for-socketpair branch May 31, 2020 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: POSIX POSIX API Library area: Samples Samples area: Sockets Networking sockets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sample and writeup for socketpair
5 participants