PATCH: properly cast c value in memset

[Leah Rowe]

Hi

Attached is a patch I did earlier, while working on something else. I
noticed that SeaBIOS's memset() implementation casts the object "s" to
(char *), which is technically incorrect; see:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/memset.html

It says:

The memset() function shall copy c (converted to an unsigned char) into
each of the first n bytes of the object pointed to by s.

The attached patch fixes this, by casting c to unsigned char, and
accordingly, casts void *s to (unsigned char *)

The current cast does (char *) which is technically incorrect, because
whether char is signed or unsigned is implementation-defined, so you
have to explicitly state signed or unsigned when signedness matters.

[Kevin O'Connor]

On Tue, Jun 06, 2023 at 02:47:04AM +0100, Leah Rowe via SeaBIOS wrote:
> Hi
>
> Attached is a patch I did earlier, while working on something else. I
> noticed that SeaBIOS's memset() implementation casts the object "s" to
> (char *), which is technically incorrect; see:
>
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/memset.html
>
> It says:
>
> The memset() function shall copy c (converted to an unsigned char) into
> each of the first n bytes of the object pointed to by s.
>
> The attached patch fixes this, by casting c to unsigned char, and
> accordingly, casts void *s to (unsigned char *)
>
> The current cast does (char *) which is technically incorrect, because
> whether char is signed or unsigned is implementation-defined, so you
> have to explicitly state signed or unsigned when signedness matters.

Thanks, but I'm not sure this makes any change in practice.  The
seabios code is heavily dependent on x86 cpus and copying an unsigned
vs signed is going to produce the same code.

Cheers,
-Kevin
>
> From 2032d33ca494d087ca7609cd18dee829173f85f6 Mon Sep 17 00:00:00 2001
> From: Leah Rowe 
> Date: Tue, 6 Jun 2023 00:52:10 +0100
> Subject: [PATCH 1/2] string: properly cast c value in memset()
>
> Per C standard, whether char is signed or unsigned
> is *implementation-defined*, but IEEE Std 1003.1-2017
> states:
>
> memset() shall copy c (converted to unsigned char) to
> each of the first n bytes of the object pointed to by
> s; if converting c to unsigned char, it also makes sense
> to cast s to unsigned char * (where the current code
> casts it to ambiguous char *)
> Signed-off-by: Leah Rowe
>
> src/string.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/string.c b/src/string.c
> index adb8198..b45565a 100644
> --- a/src/string.c
> +++ b/src/string.c
> @@ -110,7 +110,7 @@ void *
>  memset(void *s, int c, size_t n)
>  {
>      while (n)
>    ((char *)s)[--n] = c;
>    ((unsigned char *)s)[--n] = (unsigned char) c;
> return s;
>
> }
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]