socket-util: re-implement socket_address_parse_netlink() by using extract_first_word() #11740
Conversation
|
cc @mrc0mmand. |
|
It indeed does, thanks! |
|
Hmm, the issue seems to be also false-positive... |
| r = safe_atou(v[1], &group); | ||
| if (r < 0) | ||
| return r; | ||
| } |
There was a problem hiding this comment.
I think it's a good idea to replace sscanf() with something that has more strict semantics. But instead of strv_split() I'd propose using extract_first_word() to get the first word, and then safe_atou() directly on what remains. That way we don't bother with allocating an strv array that we actually don't really care much about.
|
Updated. Now |
ffa9b17 to
2700785
Compare
2700785 to
12fb22f
Compare
src/basic/socket-util.c
Outdated
| return -EINVAL; | ||
|
|
||
| word = mfree(word); | ||
| r = extract_first_word(&s, &word, NULL, 0); |
There was a problem hiding this comment.
hmm, for the second word, we can just use s directly; after all, there can only be one word left. Hence, instead of calling into extract_first_word() a second time, let's just use s right away. (note that extract_first_word() will not just return the first word, but advance s to the next word, skipping whitespace between the two words, hence this is perfect for this usecase)
There was a problem hiding this comment.
It sounds simpler, but currently it supports trailing white spaces. Simply passing the remaining string to safe_atou() drops the functionality. But I am not sure whether such a functionality is necessary or not. I will check all usecases later. Thanks.
…ract_first_word() This drops support of trailing white space when a multicast group is specified. Fixes one of issues in systemd#11738.
12fb22f to
4daa49d
Compare
|
Updated. Now the support of trailing white space when a multicast group is specified is dropped. Strictly speaking, this breaks backward compatibility of |
|
ah, the trailing whitespace issues are interesting. I wonder if we maybe should change safe_atou() and friends to ignore trailing whitespace (it already ignores leading whitespace after all). |
Hopefully fixes one of issues in #11738.