net: dns-sd: use hostname instead of instance name #89341
Conversation
github-actions
bot
requested review from
jukkar,
pdgendt,
rlubos,
ssharks and
tbursztyka
| @@ -575,6 +575,7 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |||
| size_t label_size; | |||
There was a problem hiding this comment.
The commit message talks about domain name, was that suppose to say host name?
There was a problem hiding this comment.
Yea, the terminology in the RFC is a bit confusing. Updated the commit message to be consistent
| @@ -593,9 +594,9 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |||
| /* pointer to .<Instance>.<Service>.<Protocol>.local. */ | |||
There was a problem hiding this comment.
Documentation is now wrong in this line if we replace Instance by Hostname
There was a problem hiding this comment.
Acutally it is still correct, the full service name is still in the resource record.
As described in https://www.rfc-editor.org/rfc/rfc2782, the target field of an SRV record should be the host name of the machine providing the service. Signed-off-by: Toon Stegen <toon@toostsolutions.be>
|
this fixes the tests to check the hostname is filles in the SRV record's target field. Signed-off-by: Toon Stegen <toon@toostsolutions.be>
| @@ -382,56 +382,90 @@ ZTEST(dns_sd, test_add_txt_record) | |||
| ZTEST(dns_sd, test_add_srv_record) | |||
There was a problem hiding this comment.
Typo on test commit msg
s/filles/filled/
There was a problem hiding this comment.
Please pass in hostname as a parameter to add_srv_record().
The signature of the function will need to be changed in the library as well as in the testsuite to accommodate, but overall doing so will ensure that it can be tested efficiently as a unit by parametrizing inputs.
| @@ -575,6 +575,7 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |||
| size_t label_size; | |||
| uint16_t inst_offs; | |||
| uint16_t offset = buf_offset; | |||
| const char *hostname = net_hostname_get(); | |||
There was a problem hiding this comment.
This function is already pretty testable as a unit. Is there any reason not to add hostname as a function parameter?
This change effectively hard-codes the host name, in that it is not easily manipulated by the test harness that tests add_srv_record() as a unit.
As described in https://www.rfc-editor.org/rfc/rfc2782, the target field of an SRV record should be the domain name of the target host.