[Dnsmasq-discuss] [PATCH] Fix some errors and warnings from clang-analyzer
Pali Rohár
pali.rohar at gmail.com
Thu Oct 5 09:46:38 BST 2017
On Thursday 05 October 2017 01:41:02 rosenp at gmail.com wrote:
> On Thu, 2017-10-05 at 09:48 +0200, Pali Rohár wrote:
> > On Wednesday 04 October 2017 19:22:11 Rosen Penev wrote:
> > > diff --git a/src/cache.c b/src/cache.c
> > > index 4f43246..88851e7 100644
> > > --- a/src/cache.c
> > > +++ b/src/cache.c
> > > @@ -572,7 +572,7 @@ struct crec *cache_insert(char *name, struct
> > > all_addr *addr,
> > > }
> > >
> > > if (name)
> > > - strcpy(cache_get_name(new), name);
> > > + strncpy(cache_get_name(new), name,
> > > strlen(cache_get_name(new)));
> >
> > Hi! This line looks suspicious. Should not be length argument sizeof
> > of
> > destination buffer, instead of current length of null term string?
> >
>
> Doesn't sizeof on a pointer return the size of the pointer instead of
> the string?
Yes! Therefore I wrote word construction "size of destination buffer"
and not code "sizeof(buf)". You need to take size of that destination
buffer manually.
> > Also strncpy in some circumstances fill string which is not null
> > terminated.
>
> strlen apparently does not include \0. needs a + 1 looks like.
That is another problem. But still my above sentence about strncpy
applies. strncpy does not ensure that destination string would be always
null terminated, even source one was.
--
Pali Rohár
pali.rohar at gmail.com
More information about the Dnsmasq-discuss
mailing list