[Home] [Downloads] [Search] [Help/forum]


Register forum user name Search FAQ

Gammon Forum

[Folder]  Entire forum
-> [Folder]  SMAUG
. -> [Folder]  SMAUG coding
. . -> [Subject]  returning local string

returning local string

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page


Posted by Gohan_TheDragonball   USA  (183 posts)  [Biography] bio
Date Wed 20 Jun 2007 12:15 PM (UTC)
Message
i forgot how to properly do this. i have a variable i am creating locally and returning, but i was given warnings about that, and i forget how to write the code properly to remove this warning.


char *intToRoman( int value )
{
    char result[MAX_STRING_LENGTH];

    result[0] = '\0';

    if ( value <= 0 )
        return result;

    while ( value > 0 )
    {
        if ( value >= 1000 )
        {
            strcat( result, "M" );
            value -= 1000;
        }
        else if ( value >= 900 )
        {
            strcat( result, "CM" );
            value -= 900;
        }
        else if ( value >= 500 )
        {
            strcat( result, "D" );
            value -= 500;
        }
        else if ( value >= 400 )
        {
            strcat( result, "CD" );
            value -= 400;
        }
        else if ( value >= 100 )
        {
            strcat( result, "C" );
            value -= 100;
        }
        else if ( value >= 90 )
        {
            strcat( result, "XC" );
            value -= 90;
        }
        else if ( value >= 50 )
        {
            strcat( result, "L" );
            value -= 50;
        }
        else if ( value >= 40 )
        {
            strcat( result, "XL" );
            value -= 40;
        }
        else if ( value >= 10 )
        {
            strcat( result, "X" );
            value -= 10;
        }
        else if ( value >= 9 )
        {
            strcat( result, "IX" );
            value -= 9;
        }
        else if ( value >= 5 )
        {
            strcat( result, "V" );
            value -= 5;
        }
        else if ( value >= 4 )
        {
            strcat( result, "IV" );
            value -= 4;
        }
        else
        {
            strcat( result, "I" );
            value--;
        }
    }

    return result;
}
[Go to top] top

Posted by Zeno   USA  (2,871 posts)  [Biography] bio
Date Reply #1 on Wed 20 Jun 2007 12:37 PM (UTC)
Message
I think the string you need to return needs to be static.

Zeno McDohl,
Owner of Bleached InuYasha Galaxy
http://www.biyg.org
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #2 on Wed 20 Jun 2007 05:32 PM (UTC)
Message
Yes, otherwise you are returning an address on the stack, which has the potential for getting cleared/etc.

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Isthiriel   (113 posts)  [Biography] bio
Date Reply #3 on Thu 21 Jun 2007 11:38 AM (UTC)
Message
Potentially? It is free'd as soon as the function returns so the returned pointer is dangling before it is even used :(

Declaring the retvalue as a static will avoid that problem with the proviso that you do not call the function again before using the result.

For.ex:
printf("%s - %s\n", intToRoman(3), intToRoman(49))
will produce:
XLIX - XLIX

(Shouldn't that be IL?)

HTH.
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #4 on Thu 21 Jun 2007 05:53 PM (UTC)
Message
Well, the stack frame is technically popped, but not cleared: its contents aren't necessarily wiped. So you might still find your string there. Of course, it is a Very Bad Idea to even think about relying on that.

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Nick Gammon   Australia  (22,966 posts)  [Biography] bio   Forum Administrator
Date Reply #5 on Thu 21 Jun 2007 09:43 PM (UTC)
Message
Quote:

char result[MAX_STRING_LENGTH];


You know how long that is? 4096 bytes. That is 4 Kb.

How big a roman numeral are you expecting here? That is long enough for 51 lines of 80 characters per line.

If you make that static, then the program will always occupy 4 Kb more memory than it used to. I know memory is cheap these days, but even so, I suggest working out how big a buffer is really required.

For example, a simple test on the input number (eg. not to be > 10000), and you can probably work out that the buffer only needs to be 30 bytes or so.

Actually, a quick test shows that for big numbers, the roman numeral generated gets large very quickly. For example, calculating the Roman Numeral of 10000000 generates 10000 bytes (all of them are "M").

So, for big numbers the buffer is too small, and for small ones it is much too large. For a modest number, like 2187, I got the result MMCLXXXVII. This is obviously a lot less than 4096 bytes long.

I suggest a check at the start, to limit the number to be (say) less than 10000, and then make the buffer size much smaller.

Incidentally, this is your routine converted into Lua so I could play with it:


function RomanNumerals (value)
  local result = ""
  
  if value <= 0 then
     return result
  end -- if
  
  while value > 0 do
   if value >= 1000 then
     result = result .. "M"
     value = value - 1000
   elseif value >= 900 then
     result = result .. "CM"
     value = value - 900
   elseif value >= 500 then
     result = result .. "D"
     value = value - 500
   elseif value >= 400 then
     result = result .. "CD"
     value = value - 400
   elseif value >= 100 then
     result = result .. "C"
     value = value - 100
   elseif value >= 90 then
     result = result .. "XC"
     value = value - 90
   elseif value >= 50 then
     result = result .. "L"
     value = value - 50
   elseif value >= 40 then
     result = result .. "XL"
     value = value - 40
   elseif value >= 10 then
     result = result .. "X"
     value = value - 10
   elseif value >= 9 then
     result = result .. "IX"
     value = value - 9
   elseif value >= 5 then
     result = result .. "V"
     value = value - 5
   elseif value >= 4 then
     result = result .. "IV"
     value = value - 4
   else
     result = result .. "I"
     value = value - 1
   end -- if
  end -- while
  return result
end -- function

print (RomanNumerals (19850))  --> MMMMMMMMMMMMMMMMMMMDCCCL


- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Gohan_TheDragonball   USA  (183 posts)  [Biography] bio
Date Reply #6 on Fri 22 Jun 2007 12:28 AM (UTC)
Message
i appreciate the feedback and suggestions, changing it to static cleared the warning and the function works perfectly.

heres what i got now:

char *intToRoman( int value )
{
    static char result[100];

    result[0] = '\0';

    if ( value <= 0 )
        return result;

    while ( value > 0 )
    {
        if ( value >= 1000 )
        {
            strcat( result, "M" );
            value -= 1000;
        }
        else if ( value >= 900 )
        {
            strcat( result, "CM" );
            value -= 900;
        }
        else if ( value >= 500 )
        {
            strcat( result, "D" );
            value -= 500;
        }
        else if ( value >= 400 )
        {
            strcat( result, "CD" );
            value -= 400;
        }
        else if ( value >= 100 )
        {
            strcat( result, "C" );
            value -= 100;
        }
        else if ( value >= 90 )
        {
            strcat( result, "XC" );
            value -= 90;
        }
        else if ( value >= 50 )
        {
            strcat( result, "L" );
            value -= 50;
        }
        else if ( value >= 40 )
        {
            strcat( result, "XL" );
            value -= 40;
        }
        else if ( value >= 10 )
        {
            strcat( result, "X" );
            value -= 10;
        }
        else if ( value >= 9 )
        {
            strcat( result, "IX" );
            value -= 9;
        }
        else if ( value >= 5 )
        {
            strcat( result, "V" );
            value -= 5;
        }
        else if ( value >= 4 )
        {
            strcat( result, "IV" );
            value -= 4;
        }
        else
        {
            strcat( result, "I" );
            value--;
        }
    }

    return result;
}
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #7 on Fri 22 Jun 2007 12:35 AM (UTC)
Message
Your function will at worse crash the game and at best do something bad if you are given a number that causes the numeral to be greater than 100 characters in length. I'd do what Nick suggested and add a cap on the number; if it's greater than a few tens of thousands, you might want to return an empty string or an error or something of the sort.

I imagine that since this is being printed to players you don't expect numerals greater than a few characters anyhow.

Nonetheless, it's better to insert the check: it's extremely easy to do and makes the code more bullet-proof.

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Gohan_TheDragonball   USA  (183 posts)  [Biography] bio
Date Reply #8 on Fri 22 Jun 2007 01:02 AM (UTC)
Message
yeah guess i should, currently the only place the function is called from is my automated clan headquarters creation system, the only number being injected is the clans number, which just started at 1. i didn't think to put a check in since it would take many a year to reach that high a clan number. thanks for the suggestion though.
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #9 on Fri 22 Jun 2007 01:06 AM (UTC)
Message
I guess it's a question of habits. Whenever you are dealing with fixed-size buffers, adding the safety checks is a good habit to get into. Even if for this particular function you are sure there won't be a problem, if you don't force yourself to always do it, you might forget some time when it really matters. And also, maybe later, you will let players make their own roman numerals, and forget about the number range problem, and players might start messing with it and crash the game.

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Gohan_TheDragonball   USA  (183 posts)  [Biography] bio
Date Reply #10 on Fri 22 Jun 2007 05:55 AM (UTC)
Message
now heres a wierd thing, i had lowered the size of result to 100, and i was testing it out to see how big a number would get me to 100:

static char result[100];

i tried 99,999 and got:

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMCMXCIX

now that is 104 characters, shouldn't that have technically caused a crash
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #11 on Fri 22 Jun 2007 07:18 AM (UTC)
Message
That will cause a stack overflow. The string is extending beyond its normal boundaries, overwriting whatever is after it in the data segment of the program. Whatever goes after that static string got corrupted. The crash would occur depending on when and how you access that memory, and what is inside it. Either way, something got broken one way or another.

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Nick Gammon   Australia  (22,966 posts)  [Biography] bio   Forum Administrator
Date Reply #12 on Fri 22 Jun 2007 12:25 PM (UTC)
Message
Quote:

Your function will at worse crash the game and at best do something bad if you are given a number that causes the numeral to be greater than 100 characters in length.


It is 99 actually, because the null byte at the end takes a character.

Quote:

... now that is 104 characters, shouldn't that have technically caused a crash


If you overflow the stack, even by a single byte, you are setting yourself up for trouble. Maybe your test won't crash. Maybe it won't crash for 5 minutes, or ever. But one day you will find that, a minute after someone looks at the clan number (which by that stage has gone very large for some reason), and then they do something else with that part of memory, it will crash, and you won't connect the two events.

It's really the principle of the thing. As David says, if you design the algorithm thinking "it will never be greater than 1000", then put the test in. Something like this:


if ( value > 1000 )
  {
  strcpy (result, "too big");
  return result;
  }


With weird things like Roman Numerals, I would actually write a short test program (like the Lua one), and pump through every possible number you are expecting (eg. 1 to 1000) and get the test to inform you which was the longest string it ever generated. Then make the buffer at least as long as that, plus 1 for the string terminator byte.

For example, using the Lua code above, I added this bit:


max_len = 0

for i = 1, 1000 do
  s = RomanNumerals (i)
  if #s > max_len then
    max_len = #s
    max_number = i
    max_result = s
  end -- if
end -- for

print (string.format ("Largest was %i, length %i, result was %s",
       max_number, max_len, max_result))


The result was:


Largest was 888, length 12, result was DCCCLXXXVIII


If you try 10000, the result is:


Largest was 9888, length 21, result was MMMMMMMMMDCCCLXXXVIII


- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.


28,133 views.

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page

Go to topic:           Search the forum


[Go to top] top

Quick links: MUSHclient. MUSHclient help. Forum shortcuts. Posting templates. Lua modules. Lua documentation.

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

[Home]


Written by Nick Gammon - 5K   profile for Nick Gammon on Stack Exchange, a network of free, community-driven Q&A sites   Marriage equality

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( https://gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Hosted at HostDash]