https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117023

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I wonder about the strncat case (and I guess wcsncat):
"The strncat function appends not more than n characters (a null character and
characters that follow it are not appended) from the array pointed to by s2 to
the end of the string pointed to by s1. The initial character of s2 overwrites
the null character at the end of s1. A terminating null character is always
appended to the result.378) If copying takes place between objects that
overlap, the behavior is undefined."
I don't see there any special special case for the n == 0 case, the description
clearly states that a null char is always appended.
So, I think it is just fine to call strncat (s, NULL, 0); because it will read
no more than 0 characters from src, but I think it is problematic to call
strncat (NULL, s, 0);
or strncat (NULL, NULL, 0); because it can't always append the null character
to that.
And looking at various implementations, I think they all crash too:
Linux strncat manpage:
           char *
           strncat(char *dest, const char *src, size_t n)
           {
               size_t dest_len = strlen(dest);
               size_t i;

               for (i = 0 ; i < n && src[i] != '\0' ; i++)
                   dest[dest_len + i] = src[i];
               dest[dest_len + i] = '\0';

               return dest;
           }
crashes already in strlen, but would also on the dest[dest_len + i] = '\0'.
glibc generic strncat:
char *
STRNCAT (char *s1, const char *s2, size_t n)
{
  char *s = s1;

  /* Find the end of S1.  */
  s1 += strlen (s1);

  size_t ss = __strnlen (s2, n);

  s1[ss] = '\0';
  memcpy (s1, s2, ss);

  return s;
}
likewise. And some assembly implementations too.
Thus, with the proposed nonnull_if_nonzero attribute, I think it should
actually be
__attribute__((__nonnull__(1), __nonnull_if_nonzero__(2,3)))
i.e. first argument must be always non-NULL and second argument must be
non-NULL if third argument is non-zero.

Reply via email to