[PATCH 1/2] gcc8.1: fix strncpy bounds warnings

[John Keeping]

On Sat, Jun 16, 2018 at 09:12:08PM +0800, Andy Green wrote:
> 
> 
> On June 16, 2018 9:04:48 PM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
> >On Wed, Jun 13, 2018 at 07:33:59AM +0800, Andy Green wrote:
> >> These warnings are coming on default Fedora 28 build and probably
> >others using gcc 8.1
> >> 
> >> ../shared.c: In function ‘expand_macro’:
> >> ../shared.c:483:3: warning: ‘strncpy’ specified bound depends on the
> >length of the source argument [-Wstringop-overflow=]
> >>    strncpy(name, value, len);
> >>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> ../shared.c:480:9: note: length computed here
> >>    len = strlen(value);
> >>          ^~~~~~~~~~~~~
> >> 
> >> strncpy with a computed length via strlen is usually
> >> not the right thing.
> >> 
> >> ../ui-shared.c: In function ‘cgit_repobasename’:
> >> ../ui-shared.c:135:2: warning: ‘strncpy’ specified bound 1024 equals
> >destination size [-Wstringop-truncation]
> >>   strncpy(rvbuf, reponame, sizeof(rvbuf));
> >>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> 
> >> add one char of padding and adjust so the code does the same.
> >> 
> >> Signed-off-by: Andy Green <andy at warmcat.com>
> >> ---
> >>  shared.c    |    2 +-
> >>  ui-shared.c |    7 ++++---
> >>  2 files changed, 5 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/shared.c b/shared.c
> >> index 21ac8f4..477db0a 100644
> >> --- a/shared.c
> >> +++ b/shared.c
> >> @@ -480,7 +480,7 @@ static char *expand_macro(char *name, int
> >maxlength)
> >>  		len = strlen(value);
> >>  		if (len > maxlength)
> >>  			len = maxlength;
> >> -		strncpy(name, value, len);
> >> +		memcpy(name, value, len);
> >
> >This is a change in behaviour because strncpy is guaranteed to null
> >terminate the output (even writing one beyond len if necessary) whereas
> >memcpy does not.
> 
> Eh... are you sure about that?  It's not my understanding, and --->
> 
> https://linux.die.net/man/3/strncpy
> 
> The strncpy() function is similar, except that at most n bytes of src
> are copied. Warning: If there is no null byte among the first n bytes
> of src, the string placed in dest will not be null-terminated. 

Yes, I'm getting it confused with strncat.  And in this case we do
ensure that the output is null terminated separately.