1

Essentially I am tokenizing a string and strncpying the string found to a structure member, i.e. stringid. It of course suffers from the problem of lack of termination, I have added an extra array space for it, I've no clue how to add it properly.

I had done it like so:

my_struct[iteration].stringID[ID_SIZE-1] = '\0' //updated

I am unsure if that really works, it looks horrible IMO.

Str(n)cpying a null character, or 0, results in a warning generated by GCC and MinGW:

warning: null argument where non-null required (arg 2)

Am I blind on how to do this in a clean manner? I was thinking of memsetting the member array to all zeros, and then copying the string in to nicely fit with null termination. Do you have any suggestions or practises?

Kenshin R.
  • 117
  • 1
  • 5
  • What is the size of `stringID`? If it's `ID_SIZE` then you're one past the end of the string with that assignment. – jonsca May 04 '11 at 12:09
  • I corrected the example, I was really meaning ID_SIZE-1 to access the last array. It was purely an example of how "ugly" the solution looked. The -1 makes it even more so. – Kenshin R. May 04 '11 at 12:12

4 Answers4

3

Two things:

  1. Beware that strncpy() has very unexpected semantics, it will always 0-fill the buffer if not totally filled by the string, and it will not terminate the string if it completely fills the buffer. Both of these are weird enough that I recommend against using it.
  2. Never index an array with it's size, like stringID[ID_SIZE] seems to be doing; that is out of bounds.

The best solution is to write a custom version of strncpy() that is less weird, or (if you know the length of the input) just use strcpy().

UPDATE: If the length of your input tokens is static, but they're not 0-terminated in the source buffer due to your tokenization process, then just use memcpy() and manual termination:

const char * token = ...; /* Extract from tokenization somehow. Not 0-terminated. */
const size_t token_length = ... /* Perhaps from tokenization step. */
memcpy(my_struct[iteration].stringID, token, token_length);
my_struct[iteration].stringID[token_length] = '\0';

I don't see a need to "wrap" the above in a macro.

unwind
  • 391,730
  • 64
  • 469
  • 606
  • The input length of each token is static, so it being too small and it terminating automatically is not part of my requirement, sorry for unclarity. A custom solution seems good (strlcpy I've heard of is a replacement often used) – Kenshin R. May 04 '11 at 12:18
1

Actually, null terminating the way you suggested isn't horrible at all and I personally very much like it.

The best way, in my opinion, would be to define it as a macro in similar fashion:

// for char* blah;
#define TERMINATE_DYNAMIC_STRING(str, len) str[len] = '\0';
// for char mytext[] = "hello";
#define TERMINATE_STRING(str) str[sizeof(str)/sizeof(str[0]) - 1] = '\0';

Then you can use it all around your code as much as you want.

On Windows Microsoft gives you the following functions which null terminate when copying string: StringCchCopy

Grim
  • 937
  • 10
  • 24
  • I find this answer inspiring, as I had seen it as ugly only because it looked unnecessary compared to a strcpy if it would work. Writing a macro, wrapper, or what I have chosen (leave it be) is my solution. – Kenshin R. May 04 '11 at 12:29
  • Note that the value of `sizeof` is in units of `char`, so for strings you never need to involve `sizeof` of actual elements. For this approach, I'd rewrite `TERMINATE_STRING` as just str[sizeof str - 1] = '\0'. – unwind May 04 '11 at 12:39
  • sizeof() returns the size in bytes. The code I offered will work correctly for both WCHARs and CHARs – Grim May 04 '11 at 12:57
0

As others have noted, strncpy has odd semantics. The idiomatic way to do a bounded string copy is to strncat onto an empty string:

my_struct[iteration].stringID[0] = '\0';
strncat(my_struct[iteration].stringID, src, ID_SIZE-1);

This always appends a terminating NUL, (and fills at most ID_SIZE characters including the NUL).

fizzer
  • 13,551
  • 9
  • 39
  • 61
0

I ended-up writing a strncpyz(char* pszTo, char* pszTo, size_t lSize) function that forces the NULL termination. This works pretty-well if you have a library to put it in. Using it also requires minimal code changes.

I'm not keen on the macro approach because somebody will pass a pointer to the wrong macro.

davep
  • 90
  • 2