Top Mud Sites Forum

Top Mud Sites Forum (http://www.topmudsites.com/forums/index.php)
-   MUD Coding (http://www.topmudsites.com/forums/forumdisplay.php?f=9)
-   -   returning strings (http://www.topmudsites.com/forums/showthread.php?t=403)

visko 05-11-2002 10:06 PM

I must be insane or stupid, but either way I'm going out of my mind trying to figure out what I'm doing wrong.

I've got a simple little bitty function for creating a series of asterisks, variying by clan rank. The code is as follows:

char *show_rank(CHAR_DATA *ch)
{
int i = 0;
char buf[256];

buf[0] = '[';

if (ch->rank == 0)
{ strcat(buf, "]");
return buf;}

for (i = 1; i <= ch->rank; i++)
strcat(buf, "{R*{x");

strcat(buf, "]");
return buf;
}

I'm getting the following error during compile time:
handler.c:273: warning: function returns address of local variable

Now I understand why this happens, but I can't figure out how to get around it. I haven't dealt much with passing strings between functions, so I'm a bit clueless here. Anyone got a suggestion?

-Visko

Yui Unifex 05-11-2002 10:12 PM

Your problem is just that -- you're returning the address of a local variable.  Every variable in a function is only local to that function -- if you tried to return the address of it, it would be invalid.  So, your compiler is warning you that you shouldn't do this.  Your problem arises because "char buf[256];" is actually "char *buf", which is a pointer, or an address.

You can solve this one of two ways: You can call str_dup() on the string, but you must remember to call free_string() on it!  Or, you can make the local variable static, which means that it stays alive even when you're not in the function.  To make the variable static, all you'd have to do is place "static char buf[256];" instead of the current line.

One quick design caveat: You should really return most strings as const.  I know alot of Dikurivatives don't do this, and it forces you to break const correctness elsewhere, but it's a bad practice that should be avoided.

visko 05-11-2002 10:17 PM

Huh. Thanks for that; most of the time you're not passing strings much in Diku-like muds anyway. They seemed to have gone out of their way to reference all strings to tables or put int interpreter functions that switch between numbers and letters/strings or whatever. It's cool, but it does leave certain knowledge up to the coder for special cases.

Thanks again,
-Visko

Torren 05-15-2002 09:13 AM

[code]
  for (i = 1; i <= ch->rank; i++)
  strcat(buf, "{R*{x");
[/quote]
I noticed that your using the colour codes to make each pip red. only problem with this is you might have some formatting issues down the road because the code sees that string as 3 characters but the terminal only interpets as 1. which for 1 pip isn't bad but when your talking 5 pip's thats 15 characters of which only 5 are showing. You may want to start the first bracket with [{R and then end either end bracket with {x]. So if they don't have pips it'll be blank. This isn't anything big it wont cause the universe to implode, but the Users might notice the formating problems.

KaVir 05-15-2002 09:48 AM

As Unifex points out, you're returning a pointer to a stack-based variable, which becomes invalid as soon as you exit the function.  As well as the two suggestions provided (using str_dup() to store the variable on the heap, or the static storage class specifier to allocate space for it in the data segment) there are also a couple of other possibilities which spring to mind.

The first is to add an extra argument to the function, which is a pointer to an allocated chunk of memory (which CAN be on the stack).

The second is to encapsulate the array within a structure and return the entire structure from the function (although this is rather inefficient).

Aside from the memory management issue, you're also not initialising the buf array.  You do assign a value to the first element ("["), but you don't put a NUL ('\0') character after it.  The strcat() requires an initialised string.  You could either assign the second element of buf to NUL, or add the initialisation to the declaration like so:

char buf[256] = { '\0' };

That will set every element of buf to NUL.

I'm also rather confused by Unifex's statement ""char buf[256];" is actually "char *buf"".  Unifex, would you care to elaborate on this?  "char buf[]" and "char *buf" are equivilent as function arguments, but they are certainly not the same.  Am I misreading something in your post?

Yui Unifex 05-15-2002 10:48 AM

Thanks for catching that.  I only looked at his variable declarations.

Right -- one is a pointer, the other is a pointer to a known size of a bunch chars on the stack.  The distinction I was trying to make was that they're both pointers, and that's why he's getting the "returning address of local variable" warning.  I never meant to imply that they're precisely the same, merely that they share an important characteristic =).

KaVir 05-15-2002 11:10 AM

Erm nope, they're not both pointers. One is a pointer, the other is an array. The gives a good description of this. The reason for the warning was simply the fact that he was returning the address of a local variable - however that variable could have been anything. Syntactically "buf" is the same as "&buf[0]", but that doesn't make buf a pointer...

Yui Unifex 05-15-2002 11:39 AM

Uh, yeah. Thanks for pointing that out =).


All times are GMT -4. The time now is 09:19 PM.

Powered by vBulletin® Version 3.6.7
Copyright ©2000 - 2024, Jelsoft Enterprises Ltd.
Copyright Top Mud Sites.com 2022