06 Nov, 2007, David Haley wrote in the 21st comment:
Votes: 0
Solution two won't work. Every one of your strings will be pointing to the same static buffer returned by strlower. You need to either use one strlower result at a time, or you'll need to copy the results and later free the copies. That would be simpler in C++.
06 Nov, 2007, Kayle wrote in the 22nd comment:
Votes: 0
So I'm stuck with Option one then? Fun stuff.
06 Nov, 2007, Justice wrote in the 23rd comment:
Votes: 0
Actually, both options have issues.

Option 1 will over-write the values of the buf for every call given. You can use some pointer arithmetic to avoid this issue, but you'll need to adjust your overrun limit accordingly.

Option 2 as David points out simply points at the same buffer, however can be easily modified to copy into a local buffer. In which case, you probably want to allocate a local buffer for each string you intend to use.

char eyecolor[MAX_INPUT_LENGTH];
strcpy(eyecolor, …);
06 Nov, 2007, Kayle wrote in the 24th comment:
Votes: 0
Samson pointed another option out to me. Will all be fixed if I just don't use strlower?
06 Nov, 2007, David Haley wrote in the 25th comment:
Votes: 0
Oh, I hadn't even looked at the details of option 1. Ya, what Justice said.

A local buffer is probably the best solution if you don't like dealing with the string construction piece by piece.
06 Nov, 2007, David Haley wrote in the 26th comment:
Votes: 0
Depends on what you mean by "all", but sure, if you don't use strlower, you won't be using the same buffer over and over again, which is your current problem.
06 Nov, 2007, Justice wrote in the 27th comment:
Votes: 0
Yeah, the problem will most likely be fixed if you don't use strlower. The only reason it wouldn't be fixed is if for some reason your strings are not lowercase. I took a minute to verify that none of the function calls are duplicated, and that the rest were pointing at an array. If for example, you called a function multiple times in the same call, and that function used a static buffer, it would cause the issue again.

Edit:
If the strings aren't lowercase, then it would simply appear in it's original format, rather than duplicating the issue.
06 Nov, 2007, Kayle wrote in the 28th comment:
Votes: 0
None of the rest call a static buffer, I don't tend to use static anything, which is why I had no idea what was going on. :P
06 Nov, 2007, David Haley wrote in the 29th comment:
Votes: 0
Kayle said:
I don't tend to use static anything

Well, the code you're writing is using a static buffer… :-)
char *show_appearance( CHAR_DATA * ch )
{
static char buf[MSL];


But yeah, it's pretty easy to mess up static buffers unless you know what's going on under the hood. It also destroys thread safety, but chances are that almost nobody cares about that.
06 Nov, 2007, Kayle wrote in the 30th comment:
Votes: 0
That was actually under the direction of Samson to get the compiler to shut up about returning buf.
06 Nov, 2007, David Haley wrote in the 31st comment:
Votes: 0
Well, yes, you have to do that, unless you return memory that you allocate. You can't safely return pointers to local buffers.
06 Nov, 2007, Justice wrote in the 32nd comment:
Votes: 0
It's okay, that's how you learn. This is a pretty common mistake when using printf… since you just assume functions work the way they do. I first ran into it back in 95 with the capitalize function.

Kayle said:
None of the rest call a static buffer, I don't tend to use static anything, which is why I had no idea what was going on. :P
06 Nov, 2007, Kayle wrote in the 33rd comment:
Votes: 0
snprintf( buf, MSL, "&wYour &W%s&w eyes twinkle with a need for adventure, and contrast with your\r\n",
"&W%s&w skin. Your &W%s %s&w hair is worn &W%s%s&w. You stand at &W%d'%d\"&w and weigh\r\n",
"&W%d&w pounds, with a &W%s&w build. Your &W%s%s.&D\r\n",
get_eye_color( ch, ch->eyecolor ), get_skin_tone( ch, ch->skintone ),
hair_length[ch->hlength], get_hair_color( ch, ch->hcolor ),
ch->hstyle == 9 ? "as " : "", hair_style[ch->hstyle], feet, inches,
ch->weight, build_name[ch->build], facial_hair[ch->facialhair],
ch->facialhair == 0 ? " face draws all the attention of the ladies" : ch->facialhair == 5 ? //Line: 533
" are neatly trimmed" : " is neatly trimmed" );


Quote
player.c: In function âchar* show_appearance(CHAR_DATA*)â:
player.c:533: warning: too many arguments for format
player.c:533: warning: too many arguments for format
make[1]: *** [o/player.o] Error 1
make: *** [all] Error 2


Both compilers I have access to, both on Samson's Arthmoor server, and CYGWIN on my laptop are complaining about the same thing, but I could 12 variable locations, and 12 arguments.. and I don't understand why this would crop up now, since it only started after I removed the calls to strlower, any ideas?
06 Nov, 2007, David Haley wrote in the 34th comment:
Votes: 0
You have commas in between the format lines.
06 Nov, 2007, Justice wrote in the 35th comment:
Votes: 0
Okay, here's the deal.

When a function returns, it theoretically cleans up it's local variables. If you return a pointer to a local variable, the odds are that it will become invalid, if not immediately, then very shortly. By making it static, it remains… static, that is, it shared across every execution of the function. It gets allocated and deallocated once in your program. If you have a new pointer, then it is also acceptable because it's not a local variable that according to the rules of C/C++ must get cleaned up.

It's a very very simplified description but should help you to understand what's going on.

Kayle said:
That was actually under the direction of Samson to get the compiler to shut up about returning buf.
06 Nov, 2007, Kayle wrote in the 36th comment:
Votes: 0
David: Why I didn't notice that of all things.. I don't know. I should probably stop coding when I'm tired from the recent trend of things.

Justice: That's interesting, but leads me to need to clarify something. What this function does is return a generic description displayed in the nanny at character creation, after randomly assigning values to the descriptive slots on a player, it shows them the description then offers them the option to pick their own values. Is this going to allow for the values to change? And is this going to cause problems if multiple people make characters at the same time? I'd hate to have someone think they look one way, and like it only to find out later that isn't how they look at all, but the other person creating a character at the same time.
06 Nov, 2007, David Haley wrote in the 37th comment:
Votes: 0
Kayle said:
And is this going to cause problems if multiple people make characters at the same time?

No, because they're not actually doing it at the same time as far as the code is concerned. The code isn't multi-threaded, so things are still happening one at a time even if the speed of handling gives the illusion of simultaneity.

Think of the static buffer thing like this: it's writing to a single shared string, that everybody calling the function uses. Now, if you call the function and get the returned address, it's to the shared string. If you don't use it immediately, and you later call the function again, your next result is to the same shared buffer. Actually, you shouldn't really think of the function as "returning" a result; it's just giving you the address to the buffer in which it did whatever it did. (Think of it as a shared scratch pad, I guess.)

So, this is only a problem when you call the function returning a static buffer, and need to call the same function several times on different arguments, and keep track of the results independently.
06 Nov, 2007, Kayle wrote in the 38th comment:
Votes: 0
Hmmm. Well, I'm not all that sure, but the arguments for the function shouldn't change, the only argument is the character data after all, so assuming between players that it clears said buffer, it should be fine I suppose, but I'll let you know if I have any problems with it tomorrow, when I have time to finish trouble shooting the entire process. Right now I appear to have reversed signs somewhere and weight won't actually take an argument within the given limits. >.<
06 Nov, 2007, Davion wrote in the 39th comment:
Votes: 0
Why bother with a static buffer? Descriptions are pointed to by the character so returning an allocated string shouldn't be a problem at all. You wouldn't even have to worry about clean up.

char *show_appearance( CHAR_DATA * ch )
{ char buf[MSL];
char *ptr;


ptr = str_dup(buf);
return ptr;
}


Then it whatever code…

ch->description = show_appearence();


That would pretty much be the end of it. All your character handler functions will free the data and you wont have to deal with it.
06 Nov, 2007, Justice wrote in the 40th comment:
Votes: 0
Kayle said:
Is this going to allow for the values to change?

Changing shouldn't be a problem. Basically your values are stored as members of the CHAR_DATA, the function simply interprets the value and outputs a descriptive text. The static buffer simply means that it can only work with one string at a time.

Kayle said:
And is this going to cause problems if multiple people make characters at the same time? I'd hate to have someone think they look one way, and like it only to find out later that isn't how they look at all, but the other person creating a character at the same time.

This won't be a problem, as with before, your values are stored as members of CHAR_DATA, therefor each character would have it's own data that would not interfere with any other character.

Even if the code was multi-threaded (as david describes), it wouldn't be an issue… however the static buffers would quickly become a problem.
20.0/61