Hi,
On 2020-04-23 14:36:15 +0900, Kyotaro Horiguchi wrote:
> At Thu, 23 Apr 2020 01:21:21 -0300, Ranier Vilela <ranier...@gmail.com> wrote
> in
> > Em qua., 22 de abr. de 2020 às 23:27, Kyotaro Horiguchi <
> > horikyota....@gmail.com> escreveu:
> > >
> > > - strncpy(sqlca->sqlerrm.sqlerrmc, message,
> > > sizeof(sqlca->sqlerrm.sqlerrmc));
> > > - sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1] = 0;
> > > + sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1] =
> > > '\0';
> > > + strncpy(sqlca->sqlerrm.sqlerrmc, message,
> > > sizeof(sqlca->sqlerrm.sqlerrmc) - 1);
> > >
> > > The existing strncpy then terminating by NUL works fine. I don't think
> > > there's any point in doing the reverse way. Actually
> > > sizeof(sqlca->sqlerrm.sqlerrmc) - 1 is enough for the length but the
> > > existing code is not necessarily a bug.
> > >
> > Without understanding then, why Coveriy claims bug here.
>
> Well, handling non-terminated strings with str* functions are a sign
> of bug in most cases. Coverity is very useful but false positives are
> annoying. I wonder what if we attach Coverity annotations to such
> codes.
It might be worth doing something about this, for other reasons. We have
disabled -Wstringop-truncation in 716585235b1. But I've enabled it in my
debug build, because I find it useful. The only warning we're getting
in non-optimized builds is
/home/andres/src/postgresql/src/interfaces/ecpg/ecpglib/misc.c: In function
‘ECPGset_var’:
/home/andres/src/postgresql/src/interfaces/ecpg/ecpglib/misc.c:565:17: warning:
‘strncpy’ output truncated before terminating nul copying 5 bytes from a string
of the same length [-Wstringop-truncation]
565 | strncpy(sqlca->sqlstate, "YE001",
sizeof(sqlca->sqlstate));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
One way we could address this is to use the 'nonstring' attribute gcc
has introduced, signalling that sqlca_t->sqlstate isn't zero
terminated. That removes the above warning.
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes
"The nonstring variable attribute specifies that an object or member
declaration with type array of char, signed char, or unsigned char, or pointer
to such a type is intended to store character arrays that do not necessarily
contain a terminating NUL. This is useful in detecting uses of such arrays or
pointers with functions that expect NUL-terminated strings, and to avoid
warnings when such an array or pointer is used as an argument to a bounded
string manipulation function such as strncpy. For example, without the
attribute, GCC will issue a warning for the strncpy call below because it may
truncate the copy without appending the terminating NUL character. Using the
attribute makes it possible to suppress the warning. However, when the array is
declared with the attribute the call to strlen is diagnosed because when the
array doesn’t contain a NUL-terminated string the call is undefined. To copy,
compare, of search non-string character arrays use the memcpy, memcmp, memchr,
and other functions that operate on arrays of bytes. In addition, calling
strnlen and strndup with such arrays is safe provided a suitable bound is
specified, and not diagnosed. "
I've not looked at how much work it'd be to make a recent-ish gcc not to
produce lots of false positives in optimized builds.
Greetings,
Andres Freund
