Bug 5944 - Com_sprintf not working properly
Status: RESOLVED WONTFIX
Alias: None
Product: ioquake3
Classification: Unclassified
Component: Misc
Version: GIT MASTER
Hardware: PC Linux
: P3 major
Assignee: Zachary J. Slater
QA Contact: ioquake3 bugzilla mailing list
URL:
Depends on:
Blocks:
 
Reported: 2013-05-28 10:24 EDT by signaljammed
Modified: 2013-11-09 16:51:20 EST
4 users (show)

See Also:



Description signaljammed 2013-05-28 10:24:38 EDT
In the following code, copy is built on itself one character at a time from original. This used to work in an older version of ioq3 from back in 2009, but now when I use this same method with the latest ioquake3 source code it does not build upon copy. Instead each time the Com_sprintf call is made what copy previously had is being overwritten by the *p char. It is almost like it is ignoring the %s format character in the call. 

char original[1024];
char copy[1024];
char *p;

p = original;
while (1) {
// do some text parsing

// end of line
if (!*p) break;

// All cases above fail so write out the char
Com_sprintf(copy, sizeof(copy), "%s%c", copy, *p);
p++;
}

So if the original text was something like "the fox ran over the hill" then copy would be the last character in the string 'l'.  It should be an exact replica of original.
Comment 1 signaljammed 2013-05-30 18:04:12 EDT
This seems to be a problem with the call to Q_vsnprintf() within Com_sprintf() upon further troubleshooting, however I am unsure why vsnprintf isn't handling the formatting correctly as it used to.
Comment 2 Simon McVittie 2013-05-31 03:29:05 EDT
Com_sprintf() is basically ISO C (v)snprintf(), which doesn't allow this usage:

       C99 and POSIX.1-2001 specify that the results are undefined if  a  call
       to  sprintf(), snprintf(), vsprintf(), or vsnprintf() would cause copy‐
       ing to take place between objects that overlap  (e.g.,  if  the  target
       string  array and one of the supplied input arguments refer to the same
       buffer).

If you're writing native code (stuff that ends up in the engine, or cgame/qagame/ui compiled to native code) then Com_sprintf() is just a wrapper around (v)snprintf(). If you're writing bytecode (cgame.qvm, qagame.qvm, ui.qvm), the implementation used is in bg_lib.c.
Comment 3 Ben Millwood 2013-05-31 05:12:56 EDT
I don't think it's so terrible that we don't support this use case. After all, it nearly always makes no sense: consider

    Com_sprintf(copy, sizeof(copy), "%c%s", *p, copy);

I can imagine that just filling out copy up to its size with copies of *p.
Comment 4 signaljammed 2013-05-31 11:31:12 EDT
Wow, this is really bad news to hear.  I have written ALOT of code in my mod that utilizes this technique of building/parsing char strings.  What options do I have going forward to accomplish this same use?  What other methods are there to building strings like this?
Comment 5 Tim Angus 2013-05-31 11:42:38 EDT
Unless I'm misunderstanding, strcat?
Comment 6 signaljammed 2013-05-31 12:47:46 EDT
Yes, I suppose that would work but it will require breaking everything down into multiple steps where before it could all be done on one line.  Take for instance, 

Com_sprintf(line, sizeof(line), "%s%2i ^1:^7 ", line, i);

Now in order to accomplish the same thing with strcat I would have to create a new char to do the formatting and then call strcat to append it to line...

Com_sprintf(buffer, sizeof(buffer), "%2i ^1:^7 ", i);
strcat(line, buffer);

Doable yes but just going to be a pain in the ass.  :(
Comment 7 Tim Angus 2013-05-31 13:04:20 EDT
You could also do strcpy(buffer, va("%s", ...));. Same thing really.
Comment 8 signaljammed 2013-05-31 13:16:38 EDT
Thanks.  I'll take a look at doing it that way.
Comment 9 Ben Millwood 2013-05-31 14:07:42 EDT
Another way is something along the lines of

l = strlen(copy);
Com_sprintf(&copy[l], sizeof(copy) - l, "%2i ^1:^7", ... );

Often you already know the length of copy (or can easily keep track of it), so you can skip the first step.
Comment 10 Zack Middleton 2013-11-09 16:51:20 EST
Closing issue.