[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP
Lennart Poettering
lennart at poettering.net
Mon Aug 4 09:13:12 PDT 2014
On Mon, 04.08.14 17:27, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
>
> On Mon, Aug 04, 2014 at 05:21:46PM +0200, Lennart Poettering wrote:
> > On Wed, 30.07.14 00:37, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> >
> > > > +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
> > > > + _cleanup_free_ char *s = NULL, *p = NULL;
> > > > + int r;
> > > > +
> > > > + assert_return(ifindex > 0, -EINVAL);
> > > > + assert_return(domainname, -EINVAL);
> > > > +
> > > > + if (asprintf(&p, "/run/systemd/netif/links/%d", ifindex) < 0)
> > > > + return -ENOMEM;
> > > Not terribly important, but please spell that as:
> > >
> > > char p[sizeof("/run/systemd/netif/links/") + DECIMAL_STRING_MAX(int)];
> > > snprintf(p, sizeof(p), "/run/systemd/netif/links/%d",
> > > ifindex);
> >
> > Actually, I'd even use normal sprintf() here, not snprintf(), after all,
> > we carefully sized the string anyway, hence no need to enforce the size
> > limit here. Using sprintf() here makes is
> > clear to me that the buffer was carefully sized before, so I think would
> > be preferrable...
> On my TODO list is adding snprintf_check (or snprintf_assert, or snprintf_sized,
> not sure about the name), which would wrap the snprintf in an assert
> to check that there was no truncation.
>
> > In particular since snprintf() doesn't add a trailing NULL
> > if it truncates the string, which makes the whole excercise kinda
> > pointless... an snprintf() without something like char_array_0() invoked
> > right after it always raises my eyebrows.
> Are you sure?
>
> snprintf(3) implies that the terminating byte is always written. Testing
> confirms that fwiw.
Hmm, indeed. Apparently glibc always does that. Old solaris and windows
don't however, but we don't care about that...
So I figure we can drop char_array_0() invocations at most places,
possibly even get rid of the macro definition entirely...
Lennart
--
Lennart Poettering, Red Hat
More information about the systemd-devel
mailing list