Is this C language statement a problem?

[Jimson_1]

Hi Guys,

I have a C snippet that someone here at work says could be problematic. That the copying of memory locations should not overlap.

I've never had a problem with this style on DEC C, but maybe I've been missing something.

Can anyone offer a reliable opinion on whether it is 'dangerous' code?

The snippet is:

char currStlDate[8+1];
...
memset(currStlDate, 0, sizeof (currStlDate));
...
/* Here currStlDate is "20090526" */
sprintf(currStlDate, "%s", &currStlDate[2]);

[Joseph Huber_1]

I can't see any (overlapping) memory copy in Your snippet.
???

strcpy(dst,src): must not overlap
memmove(dst,src,size): may overlap

[Jimson_1]

So the above snippet is not a problem?

Both source and dest pointers in the sprintf statement point to memory locations where dest will overwrite source.

Isn't this what memory overlapping is?

[Joseph Huber_1]

Now I see the side-effect of sprintf is what you called overlapping 'copy'.
As far I see standards say nothing about the target of sprintf (but the question may be better placed in comp.lang.c !).
In practice I see nothing against doing it.

[Hein van den Heuvel]

imho this is poor coding, and dangerous from a maintenance perspective.

- the char line should have a comment:
// zero terminated ccyymmdd or yymmdd format.

- the memset suggests that the coder wasn't sure when or how the date string will be used. It is likely to be entirely redundant, or should be replaced by: currStlDate[0] = 0

- the sprintf suggests that the coder was too lazy to make a clear 'ccyymmdd' versus 'yymmdd' position. It seems unacceptable to me to have a variable sometimes hold one flavor, sometimes the other. It should probably be replaced by:
char *currStlDate_yymmdd = &currStlDate[2];

fwiw
Hein.

[Joseph Huber_1]

Hein's correct comment on the date semantics aside,
just curious I made a test on really overlapping (not just substring shuffling) sprintf :
/*----------------------------------------*/
#include
#include

int main(){
char test[50];

strcpy(test,"abcdefghijklmnopqrstuvwxyz1234567890");

sprintf(test,"%s",test);
printf("%s\n",test);
sprintf(test," %s",&test[2]);
printf("%s\n",test);
}
/*------------------------------------------*/

One would expect the first 3 or 4 characters blanked, but the rest of the string simply copied.
On VMS/DECC: yes, it is so:
run test_sprintf
abcdefghijklmnopqrstuvwxyz1234567890
defghijklmnopqrstuvwxyz1234567890

On RH Linux/GCC: what a surprise:
./a.out
abcdefghijklmnopqrstuvwxyz1234567890
ddfghhjkllnopprsttvwxxz1224566890

[Jimson_1]

Ok I've answered my own question.

The documentation states:

int sprintf(char *s, const char *format, ...);
Equivalent to the fprintf function except that the argument s specifies
an array, rather than a stream, into which the generated output will be
written. A null character is written at the end of the characters written. If
copying takes place between objects that overlap, the behavior is undefined.
The sprintf function returns the number of characters written into the
array, not counting the terminating null character.


Thanks for your answers anyway.

James P.

[Dennis Handly]

That's correct, your sprintf is illegal, unless there is only 1 byte and a null at currStlDate[2].

The C99 Standard has the "restrict" keyword for the target buffer in sprintf.

>Isn't this what memory overlapping is?

Exactly.

>If copying takes place between objects that overlap, the behavior is undefined.

C99 is even clearer:
int sprintf(restrict char *s, const char *format, ...);

>Joseph: As far I see standards say nothing about the target of sprintf

The Standard is quite clear.

[Jimson_1]

Ok that great, thanks all for you help.

James P.

[Jimson_1]

Closed