From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: Simon Glass <sjg@chromium.org>, John Keeping <john@metanate.com>
Cc: Joel Stanley <joel@jms.id.au>,
U-Boot Mailing List <u-boot@lists.denx.de>
Subject: Re: [PATCH] image: Ensure image header name is null terminated
Date: Tue, 23 Aug 2022 16:11:56 +0200 [thread overview]
Message-ID: <dad17a9f-d823-1e8b-3381-53961294521c@prevas.dk> (raw)
In-Reply-To: <CAPnjgZ3B3KMU7_A5XktJsz+8sbDvDNjauBr4NEou5uJGeAMC4g@mail.gmail.com>
On 23/08/2022 15.38, Simon Glass wrote:
> Hi John,
>
> On Tue, 23 Aug 2022 at 03:46, John Keeping <john@metanate.com> wrote:
>>
>> On Tue, Aug 23, 2022 at 03:59:07PM +1000, Joel Stanley wrote:
>>> When building with GCC 12:
>>>
>>> ../include/image.h:779:9: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
>>> 779 | strncpy(image_get_name(hdr), name, IH_NMLEN);
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Ensure the copied string is null terminated by always setting the final
>>> byte to 0. Shorten the strncpy to IH_NMLEN-1 as we will always overwrite
>>> the last byte.
>>>
>>> We can't use strlcpy as this is code is built on the host as well as the
>>> target.
>>
>> Since this is in the header, isn't the point that it doesn't need to be
>> null-terminated?
>>
>> When printing we're careful to use:
>>
>> "%.*s", IH_NMLEN, ...
>>
>> so I think the warning is wrong here - we want both of the strncpy()
>> behaviours that are normally considered strange:
>>
>> - it's okay not to null terminate as this is an explicitly sized field
>>
>> - we want to pad the whole field with zeroes if the string is short
>
> That's my understanding too. We are careful to avoid expecting a
> terminator. I am not sure what to do with the warning though
Maybe this could be some inspiration:
info gcc
'nonstring'
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.
struct Data
{
char name [32] __attribute__ ((nonstring));
};
int f (struct Data *pd, const char *s)
{
strncpy (pd->name, s, sizeof pd->name);
...
return strlen (pd->name); // unsafe, gets a warning
}
[https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes]
next prev parent reply other threads:[~2022-08-23 14:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-23 5:59 [PATCH] image: Ensure image header name is null terminated Joel Stanley
2022-08-23 7:27 ` Wolfgang Denk
2022-08-23 9:46 ` John Keeping
2022-08-23 13:38 ` Simon Glass
2022-08-23 14:11 ` Rasmus Villemoes [this message]
2022-09-14 22:11 ` Tom Rini
2022-09-14 22:39 ` Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dad17a9f-d823-1e8b-3381-53961294521c@prevas.dk \
--to=rasmus.villemoes@prevas.dk \
--cc=joel@jms.id.au \
--cc=john@metanate.com \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.