Allow to provide multiple host names#39
Conversation
davidgraeff
force-pushed
the
dg_multi_names
branch
2 times, most recently
from
1e51de4 to
08c2393
Compare
tklauser
left a comment
tklauser
left a comment
There was a problem hiding this comment.
Thanks. Haven't looked at the PR in detail yet, a few minor comments inline.
Please also add a commit message (not just a PR description) and also add Fixes #38 to it.
I'll give this a closer look some time later this week.
davidgraeff
force-pushed
the
dg_multi_names
branch
from
4f51b53 to
b9e01b1
Compare
RFC 4795 allows an IP to be represented by not just one but multiple names. The "--hostname" / "-H" parameter can be given multiple times now. The use of a dynamic array of host names require a new public API method: `llmnr_release(void)` Keep llmnr_set_hostname function signature. This will always only update the first entry and is mainly used when no --hostname parameter has been provided and llmnrd reacts to system hostname changes. API changes: * Adapt llmnr_init to take an array of host name strings. Signed-off-by: David Graeff <david.graeff@web.de>
davidgraeff
force-pushed
the
dg_multi_names
branch
from
b9e01b1 to
c432935
Compare
tklauser
left a comment
tklauser
left a comment
There was a problem hiding this comment.
Sorry for the delay with reviewing this. Some more comments inline, mostly minor formatting nits (yes, I should add a .clang-format file 😃).
In addition to README.md, please also update the usage of the -H option (i.e. that it can be specified multiple times) in the llmnrd.1 man page and in the in-program usage text.
| size_t i; | ||
| llmnr_hostname_count = hostname_count; |
There was a problem hiding this comment.
Add an empty line after variable declaration:
| size_t i; | |
| llmnr_hostname_count = hostname_count; | |
| size_t i; | |
| llmnr_hostname_count = hostname_count; |
| llmnr_set_hostname(hostname); | ||
| size_t i; | ||
| llmnr_hostname_count = hostname_count; | ||
| llmnr_hostnames = xzalloc(hostname_count); |
There was a problem hiding this comment.
This should allocate hostname_count * sizeof(*llmnr_hostnames).
| static char llmnr_hostname[LLMNR_LABEL_MAX_SIZE + 2]; | ||
| #define LLMNR_LABEL_LEN (LLMNR_LABEL_MAX_SIZE + 2) | ||
| static char** llmnr_hostnames = NULL; | ||
| size_t llmnr_hostname_count = 0; |
There was a problem hiding this comment.
Make this static, it's not used outside the file.
| size_t i; | ||
| for(i = 0; i < llmnr_hostname_count; ++i) { |
There was a problem hiding this comment.
Add an empty line after variable declaration and add a space after the for keyword:
| size_t i; | |
| for(i = 0; i < llmnr_hostname_count; ++i) { | |
| size_t i; | |
| for (i = 0; i < llmnr_hostname_count; ++i) { |
| uint8_t n; | ||
| n = llmnr_hostnames[i][0]; |
There was a problem hiding this comment.
Assign this on declaration:
| uint8_t n; | |
| n = llmnr_hostnames[i][0]; | |
| uint8_t n = llmnr_hostnames[i][0]; |
| const uint8_t *query; | ||
| size_t query_len; | ||
| uint8_t name_len; | ||
| char* matched_hostname_entry; |
There was a problem hiding this comment.
| char* matched_hostname_entry; | |
| const char* matched_hostname; |
| /* First count given (host)names, if any, to allocate memory */ | ||
| while ((c = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) { | ||
| if (c == 'H') | ||
| ++name_count; | ||
| } |
There was a problem hiding this comment.
I don't really like the idea of iterating the options twice. But I guess there is no alternative to this :-(
|
|
||
| if (!name_count) | ||
| name_count = 1; | ||
| hostnames = xzalloc(name_count); |
There was a problem hiding this comment.
This should allocate name_count * sizeof(*hostnames).
| if (llmnrd_sock_ipv4 >= 0) | ||
| close(llmnrd_sock_ipv4); | ||
| free(hostname); | ||
| for(name_i = 0; name_i < name_count; ++name_i) { |
There was a problem hiding this comment.
Add a space after the for keyword:
| for(name_i = 0; name_i < name_count; ++name_i) { | |
| for (name_i = 0; name_i < name_count; ++name_i) { |
| log_info("Starting llmnrd on port %u, hostname %s\n", port, hostname); | ||
| log_info("Starting llmnrd on port %u. Assigned hostname(s):\n", port); | ||
|
|
||
| for(name_i = 0; name_i < name_count; ++name_i) |
There was a problem hiding this comment.
Add a space after the for keyword:
| for(name_i = 0; name_i < name_count; ++name_i) | |
| for (name_i = 0; name_i < name_count; ++name_i) |
2 participants
Allow to provide multiple host names
void llmnr_release(void);method for string array cleanupllmnr_set_hostnamefunction signature. This will always only update the first entry.llmnr_initto take an array of host name strings.Fixes #38