HPE Community read-only access December 15, 2018
This is a maintenance upgrade. You will be able to read articles and posts, but not post or reply.
Hours:
Dec 15, 4:00 am to 10:00 am UTC
Dec 14, 10:00 pm CST to Dec 15, 4:00 am CST
Dec 14, 8:00 pm PST to Dec 15, 2:00 am PST
Operating System - OpenVMS
cancel
Showing results for 
Search instead for 
Did you mean: 

Is this C language statement a problem?

 
Jimson_1
Frequent Advisor

Is this C language statement a problem?

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

9 REPLIES
Joseph Huber_1
Honored Contributor

Re: Is this C language statement a problem?

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

strcpy(dst,src): must not overlap
memmove(dst,src,size): may overlap
http://www.mpp.mpg.de/~huber
Jimson_1
Frequent Advisor

Re: Is this C language statement a problem?

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
Honored Contributor

Re: Is this C language statement a problem?

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.
http://www.mpp.mpg.de/~huber
Hein van den Heuvel
Honored Contributor

Re: Is this C language statement a problem?

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
Honored Contributor

Re: Is this C language statement a problem?

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

http://www.mpp.mpg.de/~huber
Jimson_1
Frequent Advisor

Re: Is this C language statement a problem?

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?

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
Frequent Advisor

Re: Is this C language statement a problem?

Ok that great, thanks all for you help.

James P.
Jimson_1
Frequent Advisor

Re: Is this C language statement a problem?

Closed