[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