topic Re: Is this C language statement a problem? in Operating System - OpenVMS

Is this C language statement a problem?

Jimson_1 — Tue, 26 May 2009 06:33:17 GMT

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]);

Re: Is this C language statement a problem?

Joseph Huber_1 — Tue, 26 May 2009 07:52:27 GMT

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

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

Re: Is this C language statement a problem?

Jimson_1 — Tue, 26 May 2009 09:46:17 GMT

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?

Re: Is this C language statement a problem?

Joseph Huber_1 — Tue, 26 May 2009 10:23:07 GMT

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.

Re: Is this C language statement a problem?

Hein van den Heuvel — Tue, 26 May 2009 10:34:44 GMT

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.

Re: Is this C language statement a problem?

Joseph Huber_1 — Tue, 26 May 2009 10:43:17 GMT

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

Re: Is this C language statement a problem?

Jimson_1 — Tue, 26 May 2009 10:44:34 GMT

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.

Re: Is this C language statement a problem?

Dennis Handly — Sat, 30 May 2009 15:06:04 GMT

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.

Re: Is this C language statement a problem?

Jimson_1 — Thu, 04 Jun 2009 06:12:22 GMT

Ok that great, thanks all for you help.

James P.

Re: Is this C language statement a problem?

Jimson_1 — Thu, 04 Jun 2009 06:12:41 GMT

Closed