[rbldnsd] Minor portability (non GCC compiller) enhancements to 0.966a

Michael Tokarev mjt at corpit.ru
Fri Nov 24 18:40:57 MSK 2006


Victor Duchovni wrote:

Wow.  Victor, that's quite some changes!..
I wonder why you reviewed the code in the first place... ;)
Thanks!

Comments below.

> Index: rbldnsd.c
> --- rbldnsd.c	22 Nov 2006 23:25:08 -0000	1.1.1.3
> +++ rbldnsd.c	23 Nov 2006 04:24:24 -0000
> @@ -331,7 +331,7 @@
>    for (i = 0; i < numsock; ++i) {
>      x = 65536;
>      do
> -      if (setsockopt(sock[i], SOL_SOCKET, SO_RCVBUF, &x, sizeof x) == 0)
> +      if (setsockopt(sock[i], SOL_SOCKET, SO_RCVBUF, (char *)&x, sizeof x) == 0)

Usually, setsockopt's 4th argument is a void pointer.  If it is not, the
proper cast - IMHO - is to (void*), not (char*).  Win32 SDK defines this
arg as (const char *), but casting to/from void* is usually not required.
I'd cast it to (void*).  The same (or similar) comment applies to recvfrom()
and other routines.

>          break;
>      while ((x -= (x >> 5)) >= 1024);
>    }
> @@ -509,19 +509,20 @@
>    if (!user && !(uid = getuid()))
>      user = "rbldns";
>  
> -  if (user && (p = strchr(user, ':')) != NULL)
> -    *p++ = '\0';
> -  if (!user)
> -    p = NULL;
> -  else if ((c = satoi(user)) >= 0)
> -    uid = c, gid = c;
> -  else {
> -    struct passwd *pw = getpwnam(user);
> -    if (!pw)
> -      error(0, "unknown user `%s'", user);
> -    uid = pw->pw_uid;
> -    gid = pw->pw_gid;
> -    endpwent();
> +  p = NULL;
> +  if (user) {
> +    if ((p = strchr(user, ':')) != NULL)
> +      *p++ = '\0';
> +    if ((c = satoi(user)) >= 0)
> +      uid = c, gid = c;
> +    else {
> +      struct passwd *pw = getpwnam(user);
> +      if (!pw)
> +	error(0, "unknown user `%s'", user);
> +      uid = pw->pw_uid;
> +      gid = pw->pw_gid;
> +      endpwent();
> +    }
>    }

Ok, it was quite messy, but seems to be correct.
Readability fix ;)

>    if (!uid)
>      error(0, "daemon should not run as root, specify -u option");
> Index: rbldnsd_packet.c
> --- rbldnsd_packet.c	22 Nov 2006 23:25:09 -0000	1.1.1.3
> +++ rbldnsd_packet.c	23 Nov 2006 04:06:03 -0000
> @@ -8,6 +8,7 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <netinet/in.h>
> +#include <arpa/inet.h>
>  #include <netdb.h>
>  #include <syslog.h>
>  #include "rbldnsd.h"
> @@ -982,7 +983,7 @@
>    else
>      *cp++ = '?';
>  #else
> -  strcpy(cp, inet_ntoa(((struct sockaddr_in*)pkt->p_peer)->sin_addr.s_addr));
> +  strcpy(cp, inet_ntoa(((struct sockaddr_in*)pkt->p_peer)->sin_addr));

I wonder why the compiler does not complain here - lacking the #include.
And this is a bug, too - but hopefully not affecting anything (esp.
since it's in logging routine)

> Index: rbldnsd_util.c
> --- rbldnsd_util.c	22 Nov 2006 23:25:09 -0000	1.1.1.3
> +++ rbldnsd_util.c	23 Nov 2006 04:41:29 -0000
> @@ -516,6 +516,9 @@
>    return node;
>  }
>  
> +#ifndef __GNUC__
> +#define inline
> +#endif

Umm.  I'd not do this.  Especially due to the following routine.
Alot of compilers supports `inline' keyword nowadays.  Maybe it
should be a configure test instead.  Are there any real issue(s)
with using inline?

>  /* link node to either left or right of parent,
>   * assuming both parent's links are NULL */
>  static inline void
> Index: rbldnsd_zones.c
> --- rbldnsd_zones.c	22 Nov 2006 23:25:10 -0000	1.1.1.3
> +++ rbldnsd_zones.c	23 Nov 2006 04:30:10 -0000
> @@ -215,7 +215,9 @@
>       unsigned ttl;
>  
>  #ifndef INCOMPAT_0_99
> +#ifdef __GNUC__
>  #warning NS record compatibility mode: remove for 1.0 final
> +#endif

Any compiler out there wich doesn't understand #warning?
In any case, this whole stuff is going away soon.

>       struct dsns *dsns_first = 0;
>       unsigned cnt;
>       int newformat = 0;
> @@ -529,7 +531,7 @@
>    const struct dslist *dsl;
>    { /* zone header */
>      char name[DNS_MAXDOMAIN+1];
> -    const unsigned char **nsdna = z->z_nsdna;
> +    const unsigned char * const *nsdna = z->z_nsdna;

Good catch.

Thanks!

/mjt


More information about the rbldnsd mailing list