some small bug fixes and add the possibility to build without using t…

[lherschi]

…estparm

wsd.c: With the change Dolphin shows the hostname and not only the IP. Content-Type can additionally contain one or more parameters separated by semicolon. Dolphin also sends the charset among other things.

wsdd.h: I had to remove the include because otherwise the following build error comes. error: redefinition of 'struct sockaddr_in' netinet/in.h and linux/in.h should never be used together. linux/in.h is for kernel builds and netinet/in.h is for userland builds. Also, in wsdd2.c _GNU_SOURCE is defined and thus struct ip_mreqn is available via bits/in.h.

wsdd2.c: Here my changes should be self-explanatory.

[marcosfrm]

Please list the new options in wsdd2.c's help() output and in wsdd2.8.

[lherschi]

There are no new options.

[marcosfrm]

Command line options -A and -B?

[lherschi]

Please take a closer look at the code, they are not new.

https://github.com/Netgear/wsdd2/commit/4c4b44ad1aa3661b7ac28ace70246e7b66f5761c

[marcosfrm]

Right, but they were not implemented before. 1.8.7:

wsdd2[2066]: /usr/sbin/wsdd2: invalid option -- 'A'
wsdd2[2066]: Bad option '?'
wsdd2[2066]: WSDD and LLMNR daemon
wsdd2[2066]: Usage: wsdd2 [options]
...
wsdd2[2165]: /usr/sbin/wsdd2: invalid option -- 'B'
wsdd2[2165]: Bad option '?'
wsdd2[2165]: WSDD and LLMNR daemon
wsdd2[2165]: Usage: wsdd2 [options]
...

(the Bad option message is useless btw)

wsdd2.8 needs update still.

[lherschi]

What is currently preventing the merge?

[marcosfrm]

It would be better command line options override the testparm calls. You could drop WITHOUT_TESTPARM define then. In main(), moving init_sysinfo() after command line parsing would allow that.

Is not this enough?

diff --git a/wsdd2.c b/wsdd2.c
index 8f81936..52141a3 100644
--- a/wsdd2.c
+++ b/wsdd2.c
@@ -691,9 +691,7 @@ int main(int argc, char **argv)
    const char *prog = basename(argv[0]);
    unsigned int ipv46 = 0, tcpudp = 0, llmnrwsdd = 0;

-   init_sysinfo();
-
-   while ((opt = getopt(argc, argv, "hd46utlwLWi:H:N:G:b:")) != -1) {
+   while ((opt = getopt(argc, argv, "hd46utlwLWi:H:A:N:B:G:b:")) != -1) {
        switch (opt) {
        case 'h':
            help(prog, EXIT_SUCCESS, NULL);
@@ -740,10 +738,18 @@ int main(int argc, char **argv)
            if (optarg != NULL && strlen(optarg) > 0)
                hostname = strdup(optarg);
            break;
+       case 'A':
+           if (optarg != NULL && strlen(optarg) > 0)
+               hostaliases = strdup(optarg);
+           break;
        case 'N':
            if (optarg != NULL && strlen(optarg) > 0)
                netbiosname = strdup(optarg);
            break;
+       case 'B':
+           if (optarg != NULL && strlen(optarg) > 0)
+               netbiosaliases = strdup(optarg);
+           break;
        case 'G':
            if (optarg != NULL && strlen(optarg) > 0)
                workgroup = strdup(optarg);
@@ -754,7 +760,7 @@ int main(int argc, char **argv)
                    help(prog, EXIT_FAILURE, "Bad key:val '%s'", optarg);
            break;
        case '?':
-           if (strchr("iHNGb", optopt))
+           if (strchr("iHANBGb", optopt))
                printf("Option -%c requires an argument.\n", optopt);
            /* ... fall through ... */
        default:
@@ -765,6 +771,8 @@ int main(int argc, char **argv)
    if (argc > optind)
        help(prog, EXIT_FAILURE, "Unknown argument '%s'", argv[optind]);

+   init_sysinfo();
+
    if (!ipv46)
        ipv46 = _4 | _6;
    if (!llmnrwsdd)

[marcosfrm]

Sorry, init_sysinfo() needs changes:

diff --git a/wsdd2.c b/wsdd2.c
index 8f81936..1e716a8 100644
--- a/wsdd2.c
+++ b/wsdd2.c
@@ -656,24 +656,35 @@ static void init_sysinfo()
 {
    char hostn[HOST_NAME_MAX + 1];

-   if (!hostname && gethostname(hostn, sizeof(hostn) - 1) != 0)
-       err(EXIT_FAILURE, "gethostname");
-
-   char *p = strchr(hostn, '.');
-   if (p) *p = '\0';
-   hostname = strdup(hostn);
+   /* command line options have preference */
+   if (!hostname) {
+       if (gethostname(hostn, sizeof(hostn) - 1) != 0)
+           err(EXIT_FAILURE, "gethostname");
+
+       char *p = strchr(hostn, '.');
+       if (p) *p = '\0';
+       hostname = strdup(hostn);
+   }

-   if (!hostaliases && !(hostaliases = get_smbparm("additional dns hostnames", "")))
-       err(EXIT_FAILURE, "get_smbparm");
+   if (!hostaliases) {
+       if (!(hostaliases = get_smbparm("additional dns hostnames", "")))
+           err(EXIT_FAILURE, "get_smbparm");
+   }

-   if (!netbiosname && !(netbiosname = get_smbparm("netbios name", hostname)))
-       err(EXIT_FAILURE, "get_smbparm");
+   if (!netbiosname) {
+       if (!(netbiosname = get_smbparm("netbios name", hostname)))
+           err(EXIT_FAILURE, "get_smbparm");
+   }

-   if (!netbiosaliases && !(netbiosaliases = get_smbparm("netbios aliases", "")))
-       err(EXIT_FAILURE, "get_smbparm");
+   if (!netbiosaliases) {
+       if (!(netbiosaliases = get_smbparm("netbios aliases", "")))
+           err(EXIT_FAILURE, "get_smbparm");
+   }

-   if (!workgroup && !(workgroup = get_smbparm("workgroup", "WORKGROUP")))
-       err(EXIT_FAILURE, "get_smbparm");
+   if (!workgroup) {
+       if (!(workgroup = get_smbparm("workgroup", "WORKGROUP")))
+           err(EXIT_FAILURE, "get_smbparm");
+   }

    init_getresp();
 }
@@ -691,9 +702,7 @@ int main(int argc, char **argv)
    const char *prog = basename(argv[0]);
    unsigned int ipv46 = 0, tcpudp = 0, llmnrwsdd = 0;

-   init_sysinfo();
-
-   while ((opt = getopt(argc, argv, "hd46utlwLWi:H:N:G:b:")) != -1) {
+   while ((opt = getopt(argc, argv, "hd46utlwLWi:H:A:N:B:G:b:")) != -1) {
        switch (opt) {
        case 'h':
            help(prog, EXIT_SUCCESS, NULL);
@@ -740,10 +749,18 @@ int main(int argc, char **argv)
            if (optarg != NULL && strlen(optarg) > 0)
                hostname = strdup(optarg);
            break;
+       case 'A':
+           if (optarg != NULL && strlen(optarg) > 0)
+               hostaliases = strdup(optarg);
+           break;
        case 'N':
            if (optarg != NULL && strlen(optarg) > 0)
                netbiosname = strdup(optarg);
            break;
+       case 'B':
+           if (optarg != NULL && strlen(optarg) > 0)
+               netbiosaliases = strdup(optarg);
+           break;
        case 'G':
            if (optarg != NULL && strlen(optarg) > 0)
                workgroup = strdup(optarg);
@@ -754,7 +771,7 @@ int main(int argc, char **argv)
                    help(prog, EXIT_FAILURE, "Bad key:val '%s'", optarg);
            break;
        case '?':
-           if (strchr("iHNGb", optopt))
+           if (strchr("iHANBGb", optopt))
                printf("Option -%c requires an argument.\n", optopt);
            /* ... fall through ... */
        default:
@@ -765,6 +782,8 @@ int main(int argc, char **argv)
    if (argc > optind)
        help(prog, EXIT_FAILURE, "Unknown argument '%s'", argv[optind]);

+   init_sysinfo();
+
    if (!ipv46)
        ipv46 = _4 | _6;
    if (!llmnrwsdd)

[marcosfrm]

help() output will show defaults for -H -A -N -B -G as (null). I do not think run testparm/gethostname() only to show the help text is reasonable. I vote to axe all these strings from help() since they are already documented in the man page.

[lherschi]

However, on systems without testparm I am forced to specify a hostalias.

Furthermore, I do not understand the meaning of the following amendment.

-   if (!hostaliases && !(hostaliases = get_smbparm("additional dns hostnames", "")))
-       err(EXIT_FAILURE, "get_smbparm");
+   if (!hostaliases) {
+       if (!(hostaliases = get_smbparm("additional dns hostnames", "")))
+           err(EXIT_FAILURE, "get_smbparm");
+   }

With double amperesand, the second condition is no longer evaluated if the first one already fails.

[marcosfrm]

However, on systems without testparm I am forced to specify a hostalias.

Is it not enough?

Sure, you can keep the short-circuit evaluation. It is my personal preference only.

[lherschi]

What does "not enough" mean? If I don't need an alias, I don't want to have to make one up.

[marcosfrm]

Sorry, I misread. Right, need the #define. Scratch my suggestions, you patch is minimal and works. WITHOUT_TESTPARM could be documented in README.md.

[oldium]

I just faced the issue with missing -A and -B options, so I think it would be good to resolve it somehow.

Regarding the WITHOUT_TESTPARM option. It does not change build dependencies/libraries, so I think that having this as a command-line option (to omit using testparm to query values) would make more sense - any user could use it without recompiling the tool.

[lherschi]

I will no longer make any changes to the pull request. I have been waiting for the commit for a year and three months now and have made all the "required" changes and extensions during this time. Either it will now be committed or not.

[oldium]

I will do it, no problem. Anyway, I plan to enhance the logic to accept config file value from smb.conf to read a different configuration file (if it exists) and also to allow specifying the configuration file as a command-line parameter. This should fix issue I have. I will also add logic to omit using testparm (plus add missing -A and -B handling), which is a workaround for my issue too.

[lherschi]

I chose the build-switch for testparm because I want to build it for an embedded system. If a command line switch is used, I always have to set this switch on a system on which there is definitely no testparm. This is exactly what I wanted to avoid.

What functionality is missing from -A and -B? I have already done a lot in this regard in my PR.

[oldium]

I chose the build-switch for testparm because I want to build it for an embedded system. If a command line switch is used, I always have to set this switch on a system on which there is definitely no testparm. This is exactly what I wanted to avoid.

So just fix usage of testparm - in case the hostname or netbiosname is specified on the command line, make testparm optional (i.e. call it only when it is found).

What functionality is missing from -A and -B? I have already done a lot in this regard in my PR.

Just what you did in this PR - add them to getopt. I expect that when I am finished with my changes, this PR will not be necessary.

[oldium]

Please find here result of 3-hours programming session, it should have everything I wanted (including handling of missing testparm), but I would like to test it more before I create a Pull Request (it is after midnight after all). The original code did not care much about freeing the allocated strings memory, but I kept it like that.

[lherschi]

I'll do a build test in my cross-compiler environment in the next few days.

[oldium]

I gave it a quick test and deployed a fix. I will not have time till next week to test it more I wonder.

[oldium]

I made a draft Pull Request #51 with all the changes