1

(I've seen questions 19202368, 40095973 and 1775403)

I have this:

char data[32];
memset(data, '\0', sizeof(data));
snprintf(data, sizeof(data), "%s - %d", aCharArray != NULL ? aCharArray : "", anInt);

which yields this warning when compiling on some compilers/architectures: warning: argument to 'sizeof' in 'int snprintf(char*, size_t, const char*, ...)' call is the same expression as the destination; did you mean to provide an explicit length? [-Wsizeof-pointer-memaccess]

Both aCharArray and anInt may be local to the function or passed as arguments. This is a generic example and this is the generic approach I use (using memset to initialize and preferring snprintf to sprintf). I know that if data is local I can use the same size I used to declare it but I prefer to only specify the size in the declaration, once; this way it's easier to change in the future.

Also, snprintf will avoid overflows and put the \0 at the end. strlen will work for a char* passed as argument to the function but it'll be pointless on a freshly initialized or empty (i.e. "") char[].

So, I'm not providing the string length as it may be 0, but I do want to provide the array's size.

Is this approach correct or am I missing some caveat?

On a side note, what's the compiler flag to identify switched parameters? I.e., using the above:

snprintf(data, sizeof(data), "%s - %d", anInt, aCharArray);
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
vesperto
  • 804
  • 1
  • 6
  • 26
  • 2
    There isn't much point in initialising via memset when you are immediately going to overwrite it with something else. Later C standards also have other mechanisms for initialising arrays at the point of declaration (e.g. `char data[32] = { 0 };`) – Dave Meehan May 02 '22 at 16:18
  • Which version of GCC are you using (`gcc --version` will tell you)? If `data` is an argument to a function, using `sizeof(data)` is wrong — you need to pass both the buffer and its size (`function(…, data, sizeof(data), …)` in the call, and `function(…, char *data, size_t datalen, …)` in the definition, using `datalen` in the call to `snprintf()`. If `data` is defined in the function calling `snprintf()` as shown in the question, there should not be any problem — hence the question about the version of GCC. – Jonathan Leffler May 02 '22 at 16:22
  • Yeah, I can't reproduce this. I suspect you edited your code before posting it here and changed some important details. – David Grayson May 02 '22 at 16:32
  • 2
    Using `-Wall -Wextra -Werror` will catch missequenced arguments for the format string (when the format string is a literal, of course). The explicit name is probably `-Wformat`. Check [GCC warning options](https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Warning-Options.html#Warning-Options) – Jonathan Leffler May 02 '22 at 16:33
  • You really need to provide an MCVE ([Minimal, Complete, Verifiable Example](https://stackoverflow.com/help/mcve) — or MRE or whatever name SO now uses) or an SSCCE ([Short, Self-Contained, Correct Example](http://sscce.org/)) — the same idea by a different name. – Jonathan Leffler May 02 '22 at 16:53
  • Re “This is a generic example…”: Wrong, wrong, wrong. Readers need a **specific** example that actually reproduces the problem. It is fine to prepare an example that simplifies your original code, but it needs to actually reproduce the problem. Did you test whether it does? And it needs to be complete; it needs to be code that other people can use to reproduce the problem without making any changes or additions. – Eric Postpischil May 02 '22 at 17:15

2 Answers2

1

I think it is warning you that data (as used in first param) is the same as data in the sizeof, and that data could be a pointer, not an array (depending on scope).

When it's an array you will get sizeof(char) * count of elements, but if it's a pointer, you will get sizeof(char *).

This is a common source of bugs, partly because it is common to see expressions like sizeof(buffer) / sizeof(buffer[0]) to get the maximum number of elements in an array. It will compile whether buffer is char buffer[n] or char *buffer, but the result is different.

Check your compiler docs to see how to suppress specific warnings if you are satisfied, although you can probably restructure the code also (make the array size a #define for example).

I don't understand identify switched parameters in your side note, you need to put the params in the same order as they are in the format string.

Dave Meehan
  • 3,133
  • 1
  • 17
  • 24
1

The warning is caused by passing the same expression to snprintf for the first argument and the argument to sizeof for the second. It is produced when the same pointer is used this way.

Contrary to what you posted, data is a pointer, not an array in the offending code.

This warning is a life saver, but the wording is catastrophic. It is not produced if data is an array, yet the text would still seem to apply:

 argument to 'sizeof' in 'int snprintf(char*, size_t, const char*, ...)' call is the same expression as the destination; did you mean to provide an explicit length? [-Wsizeof-pointer-memaccess]

Passing an explicit length is indeed less consistent than using sizeof(array), but sizeof(array) is more risky because it is not obvious if array is an actual array or a pointer at the call site. This warning addresses this issue for standard functions, but alas it does not seem possible to enable it for other functions.

Here is the explanation from the gcc documentation:

-Wsizeof-pointer-memaccess

Warn for suspicious length parameters to certain string and memory built-in functions if the argument uses sizeof. This warning triggers for example for memset(ptr, 0, sizeof(ptr)); if ptr is not an array, but a pointer, and suggests a possible fix, or about memcpy(&foo, ptr, sizeof(&foo));. -Wsizeof-pointer-memaccess also warns about calls to bounded string copy functions like strncat or strncpy that specify as the bound a sizeof expression of the source array. For example, in the following function the call to strncat specifies the size of the source string as the bound. That is almost certainly a mistake and so the call is diagnosed.


void make_file(const char *name)
{
 char path[PATH_MAX];
 strncpy (path, name, sizeof path - 1);
 strncat (path, ".text", sizeof ".text");
 …
}

The -Wsizeof-pointer-memaccess option is enabled by -Wall.

-Wsizeof-array-argument

Warn when the sizeof operator is applied to a parameter that is declared as an array in a function definition. This warning is enabled by default for C and C++ programs.

To allow the compiler to type check arguments to the printf and scanf families of functions, you should pass -Wall -Wextra -Werror for gcc and -Weverything -Werror for clang.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • I think the sanest approach is to have a bunch of `#define`s with default values and use those for sizes. It's still manual, but probably preferable. Tbh even if memset only uses `sizeof(char)` or `sizeof(char*)` it still _mostly_ performs the task of initializing, although by then i'd rather use `char myarray[16] = 0;` and be done with it. The corollary being only to use safe functions later, like `snprintf` with adds `\0`, – vesperto May 03 '22 at 11:03