[Dnsmasq-discuss] Announce: dnsmasq-2.77rc4

Discussion:

Simon Kelley

2017-05-20 20:50:51 UTC

I've just mase the fourth, and probably final, release candidate for
dnsmasq-27. Please download, compile and run, and report any problems
ASAP. If all looks OK, 2.77 will happen in the next week.

http://www.thekelleys.org.uk/dnsmasq/release-candidates/dnsmasq-2.77rc4.tar.gz

Cheers,

Simon.

Matthias Andree

2017-05-21 10:49:58 UTC

Permalink

Post by Simon Kelley
I've just mase the fourth, and probably final, release candidate for
dnsmasq-27. Please download, compile and run, and report any problems
ASAP. If all looks OK, 2.77 will happen in the next week.
http://www.thekelleys.org.uk/dnsmasq/release-candidates/dnsmasq-2.77rc4.tar.gz

GCC 5 and 6 also have this complaint, and they are right, unless you
prove that the address family is either AF_INET or AF_INET6.
If the latter isn't true, memcpy() copies to whichever garbage address
was in the stack and picked up by [auto] void *addrp.

edns0.c: In function 'calc_subnet_opt':
edns0.c:350:7: warning: 'addrp' may be used uninitialized in this
function [-Wmaybe-uninitialized]
memcpy(opt->addr, addrp, len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Cheers,
Matthias

Simon Kelley

2017-05-21 21:24:04 UTC

Permalink

I think my code has beaten the compiler.

opt->source_netmask will remain set to zero on any code path which
doesn't set addrp and inhibit the execution of the memcpy()

Nevertheless I'll add a check, it took me 10 minutes to conince myself
the able was true.

Cheers,

Simon.

Post by Matthias Andree

GCC 5 and 6 also have this complaint, and they are right, unless you
prove that the address family is either AF_INET or AF_INET6.
If the latter isn't true, memcpy() copies to whichever garbage address
was in the stack and picked up by [auto] void *addrp.
edns0.c:350:7: warning: 'addrp' may be used uninitialized in this
function [-Wmaybe-uninitialized]
memcpy(opt->addr, addrp, len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Cheers,
Matthias
_______________________________________________
Dnsmasq-discuss mailing list
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Matthias Andree

2017-05-22 18:46:56 UTC

Permalink

Post by Simon Kelley
I think my code has beaten the compiler.
opt->source_netmask will remain set to zero on any code path which
doesn't set addrp and inhibit the execution of the memcpy()

Wasn't obvious to me, and chances are that clang either didn't check, or
could (other than GCC) prove that it was safe.

Post by Simon Kelley
Nevertheless I'll add a check, it took me 10 minutes to conince myself
the able was true.

:-)

Matthias Andree

2017-05-21 10:38:13 UTC

Permalink

Three issues during compilation on FreeBSD 10.3 amd64, which look like
genuine bugs, and the forward.c seem critical
- these should likely be "server->flags & SERV_DO_DNSSEC), with & not &&.

Post by Simon Kelley
forward.c:902:60: warning: use of logical '&&' with constant operand
[-Wconstant-logical-operand]
option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags && SERV_DO_DNSSEC),
NULL, NULL);
^ ~~~~~~~~~~~~~~
forward.c:902:60: note: use '&' for a bitwise operation
option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags && SERV_DO_DNSSEC),
NULL, NULL);
^~
&
forward.c:902:60: note: remove constant to silence this warning
option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags && SERV_DO_DNSSEC),
NULL, NULL);
~^~~~~~~~~~~~~~~~~
forward.c:1480:59: warning: use of logical '&&' with constant operand
[-Wconstant-logical-operand]
option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags && SERV_DO_DNSSEC),
NULL, NULL);
^ ~~~~~~~~~~~~~~
forward.c:1480:59: note: use '&' for a bitwise operation
option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags && SERV_DO_DNSSEC),
NULL, NULL);
^~
&
forward.c:1480:59: note: remove constant to silence this warning
option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags && SERV_DO_DNSSEC),
NULL, NULL);
~^~~~~~~~~~~~~~~~~
rfc2131.c:839:25: warning: data argument not used by format string
[-Wformat-extra-args]
service->basename, layer);
^

I think you may want to resolve these before 2.77 release time.

And when trying to strip down the build disabling all options I expose
in FreeBSD, I get these, which are cosmetic and, in the option.c case,
are bloat.

The attached patch series (against Git master) fixes these. Use git am
to apply them in given order, and carefully review 0003* whether the
option_len() macro needs to be changed instead.

Post by Simon Kelley
option.c:910:23: warning: unused function 'add_rev6' [-Wunused-function]
static struct server *add_rev6(struct in6_addr *addr, int msize)
^
network.c:936:24: warning: unused variable 'l6' [-Wunused-variable]
struct listener *l, *l6;
^

You may consider resolving these before 2.77 with proper #ifdefs. I
don't care much.

Simon Kelley

2017-05-21 21:49:54 UTC

Permalink

Post by Matthias Andree

Three issues during compilation on FreeBSD 10.3 amd64, which look like
genuine bugs, and the forward.c seem critical
- these should likely be "server->flags & SERV_DO_DNSSEC), with & not &&.

Yuck. those came from an external patch, but I should have caught them.

Post by Matthias Andree

Taking the patch for this reluctantly. The original code seems better to
me. Pity is confuses the compiler.

Post by Matthias Andree
I think you may want to resolve these before 2.77 release time.
And when trying to strip down the build disabling all options I expose
in FreeBSD, I get these, which are cosmetic and, in the option.c case,
are bloat.
The attached patch series (against Git master) fixes these. Use git am
to apply them in given order, and carefully review 0003* whether the
option_len() macro needs to be changed instead.

You may consider resolving these before 2.77 with proper #ifdefs. I
don't care much.

I have a policy of leaving such: why complicate the code even more than
the ifdefs already do?

Cheers,

Simon.

Post by Matthias Andree
_______________________________________________
Dnsmasq-discuss mailing list
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Matthias Andree

2017-05-22 18:43:36 UTC

Permalink

Post by Simon Kelley
rfc2131.c:839:25: warning: data argument not used by format string

Post by Matthias Andree

Post by Simon Kelley
[-Wformat-extra-args]
service->basename, layer);
^

Taking the patch for this reluctantly. The original code seems better to
me. Pity is confuses the compiler.

It does not "confuse the compiler", there is an inconsistency.
-Wno-format-extra-args would avoid it.

Post by Simon Kelley

You may consider resolving these before 2.77 with proper #ifdefs. I
don't care much.

I have a policy of leaving such: why complicate the code even more than
the ifdefs already do?

Alright, perhaps -Wno-unused-variable should then be added late to the
options.

Matthias Andree

2017-05-21 11:02:38 UTC

Permalink

Finally, IDN2 and all-i18n do not appear to get along well, because
i18n insists on IDN.

Simon Kelley

2017-05-21 21:46:54 UTC

Permalink

Post by Matthias Andree

Finally, IDN2 and all-i18n do not appear to get along well, because
i18n insists on IDN.

I've removed the automatic enable IDN with i18n. Doesn't make sense when
there are options for IDN.

Cheers,

Simon.