Pages:<< prev 1 next >>
#1 Jan 10, 2011 8:58 pm
Magician
GroupMembers
Posts239
JoinedJun 13, 2008
So, greetings everyone! Been a while.
Updated my host, brand spanking new gcc 4.4.3! Now, as you all can imagine some new warnings are popping up after the old ones were cleared out.
I thought I'd pick a few warnings and see what the community says. As far as I know, most of these functions should be fairly stock(?):
swskills.c function do_bribe warning : "percent" may be used uninitialized in this function
Here is another one this time slightly different as it has to deal with formatting
track.c function found_prey warning: format not a string literal and no format arguments
Kinda stumped on these two. There are quite a few examples of these types of warnings as well.
Thanks,
ayuri
Updated my host, brand spanking new gcc 4.4.3! Now, as you all can imagine some new warnings are popping up after the old ones were cleared out.
I thought I'd pick a few warnings and see what the community says. As far as I know, most of these functions should be fairly stock(?):
swskills.c function do_bribe warning : "percent" may be used uninitialized in this function
void do_bribe ( CHAR_DATA *ch , char *argument ) { char arg1 [MAX_INPUT_LENGTH]; CHAR_DATA *victim; PLANET_DATA *planet; CLAN_DATA *clan; int percent, amount; if ( IS_NPC(ch) || !ch->pcdata || !ch->pcdata->clan || !ch->in_room->area || !ch->in_room->area->planet ) { send_to_char( "What would be the point of that.\n\r", ch ); return; } argument = one_argument( argument, arg1 ); if ( ch->mount ) { send_to_char( "You can't do that while mounted.\n\r", ch ); return; } if ( argument[0] == '\0' ) { send_to_char( "Bribe who how much?\n\r", ch ); return; } amount = atoi( argument ); if ( ( victim = get_char_room( ch, arg1 ) ) == NULL ) { send_to_char( "They aren't here.\n\r", ch ); return; } if ( victim == ch ) { send_to_char( "That's pointless.\n\r", ch ); return; } if ( IS_SET( ch->in_room->room_flags, ROOM_SAFE ) ) { set_char_color( AT_MAGIC, ch ); send_to_char( "This isn't a good place to do that.\n\r", ch ); return; } if ( amount > ch->gold) { send_to_char( "Perhaps if you had that many credits...\n\r", ch); return; } if ( amount <= 0 ) { send_to_char( "A little bit more money would be a good plan.\n\r", ch ); return; } if ( amount > 10000) { send_to_char( "You cannot bribe for more than 10000 credits at a time.\n\r", ch); return; } if ( ch->position == POS_FIGHTING ) { send_to_char( "Interesting combat technique.\n\r" , ch ); return; } if ( victim->position == POS_FIGHTING ) { send_to_char( "They're a little busy right now.\n\r" , ch ); return; } if ( ch->position <= POS_SLEEPING ) { send_to_char( "In your dreams or what?\n\r" , ch ); return; } if ( victim->position <= POS_SLEEPING ) { send_to_char( "You might want to wake them first...\n\r" , ch ); return; } if ( victim->vip_flags == 0 ) { send_to_char( "Diplomacy would be wasted on them.\n\r" , ch ); return; } ch->gold -= amount; victim->gold += amount; ch_printf( ch, "You give them a small gift on behalf of %s.\n\r", ch->pcdata->clan->name ); act( AT_ACTION, "$n offers you a small bribe.\n\r", ch, NULL, victim, TO_VICT ); act( AT_ACTION, "$n gives $N some money.\n\r", ch, NULL, victim, TO_NOTVICT ); if ( !IS_NPC( victim ) ) return; WAIT_STATE( ch, skill_table[gsn_bribe]->beats ); //LINE BELOW HERE IS WARNING warning: "percent" may be used uninitialized in this function if ( percent - amount + victim->top_level > ch->pcdata->learned[gsn_bribe] ) return; if ( ( clan = ch->pcdata->clan->mainclan ) == NULL ) clan = ch->pcdata->clan; planet = ch->in_room->area->planet; if ( clan == planet->governed_by ) { planet->pop_support += URANGE( 0.1 , amount/1000 , 2 ); send_to_char( "Popular support for your organization increases slightly.\n\r", ch ); amount = UMIN( amount ,( exp_level(ch->skill_level[POLITICIAN_ABILITY]+1) - exp_level(ch->skill_level[POLITICIAN_ABILITY]) ) ); gain_exp(ch, amount , POLITICIAN_ABILITY); ch_printf( ch , "You gain %d diplomacy experience.\n\r", amount ); learn_from_success( ch, gsn_bribe ); } if ( planet->pop_support > 100 ) planet->pop_support = 100; }
Here is another one this time slightly different as it has to deal with formatting
track.c function found_prey warning: format not a string literal and no format arguments
void found_prey( CHAR_DATA *ch, CHAR_DATA *victim ) { char buf[MAX_STRING_LENGTH]; char victname[MAX_STRING_LENGTH]; if (victim == NULL) { bug("Found_prey: null victim", 0); return; } if (ch == NULL) { bug("Found_prey: null ch", 0); return; } if ( victim->in_room == NULL ) { bug( "Found_prey: null victim->in_room", 0 ); return; } // THIS LINE BELOW is tossing warning: format not a string literal and no format arguments sprintf( victname, IS_NPC( victim ) ? victim->short_descr : race_table[victim->race].race_name ); if ( !can_see(ch, victim) ) { if ( number_percent( ) < 90 ) return; switch( number_bits( 2 ) ) { case 0: sprintf( buf, "Don't make me find you, %s!", victname ); do_say( ch, buf ); break; case 1: act( AT_ACTION, "$n sniffs around the room for $N.", ch, NULL, victim, TO_NOTVICT ); act( AT_ACTION, "You sniff around the room for $N.", ch, NULL, victim, TO_CHAR ); act( AT_ACTION, "$n sniffs around the room for you.", ch, NULL, victim, TO_VICT ); sprintf( buf, "I can smell your blood!" ); do_say( ch, buf ); break; case 2: sprintf( buf, "I'm going to tear %s apart!", victname ); do_yell( ch, buf ); break; case 3: do_say( ch, "Just wait until I find you..."); break; } return; } if ( IS_SET( ch->in_room->room_flags, ROOM_SAFE ) ) { if ( number_percent( ) < 90 ) return; switch( number_bits( 2 ) ) { case 0: do_say( ch, "C'mon out, you coward!" ); sprintf( buf, "%s is a bloody coward!", victname ); do_yell( ch, buf ); break; case 1: sprintf( buf, "Let's take this outside, %s", victname ); do_say( ch, buf ); break; case 2: sprintf( buf, "%s is a yellow-bellied wimp!", victname ); do_yell( ch, buf ); break; case 3: act( AT_ACTION, "$n takes a few swipes at $N.", ch, NULL, victim, TO_NOTVICT ); act( AT_ACTION, "You try to take a few swipes $N.", ch, NULL, victim, TO_CHAR ); act( AT_ACTION, "$n takes a few swipes at you.", ch, NULL, victim, TO_VICT ); break; } return; } switch( number_bits( 2 ) ) { case 0: sprintf( buf, "Your blood is mine, %s!", victname ); do_yell( ch, buf); break; case 1: sprintf( buf, "Alas, we meet again, %s!", victname ); do_say( ch, buf ); break; case 2: sprintf( buf, "What do you want on your tombstone, %s?", victname ); do_say( ch, buf ); break; case 3: act( AT_ACTION, "$n lunges at $N from out of nowhere!", ch, NULL, victim, TO_NOTVICT ); act( AT_ACTION, "You lunge at $N catching $M off guard!", ch, NULL, victim, TO_CHAR ); act( AT_ACTION, "$n lunges at you from out of nowhere!", ch, NULL, victim, TO_VICT ); } stop_hunting( ch ); set_fighting( ch, victim ); multi_hit(ch, victim, TYPE_UNDEFINED); return; }
Kinda stumped on these two. There are quite a few examples of these types of warnings as well.
Thanks,
ayuri
#2 Jan 11, 2011 10:13 pm
Off the Edge of the Map
GroupAdministrators
Posts1,200
JoinedMar 21, 2006
With no time to probe the track thing I don't have an answer for that one, but the bribe bit is actually a legitimate bug. Percent is never set to anything. It really is uninitialized. When I get some time tomorrow, I'll sit down here and try and give you a better explanation, and have a look at the track thing.
#3 Jan 11, 2011 11:29 pm
Magician
GroupMembers
Posts189
JoinedSep 5, 2010
In order to fix the percent thing, do the following:
Somewhere above the ifcheck with 'percent' in it, add the following:
I'm sure Kayle could give a better explanation other than the fact that you never gave 'percent' a value, which means the code uses NULL, which will crash the mud in this particular instance with the use of bribe, if it ever got past the compile sequence. heh
Somewhere above the ifcheck with 'percent' in it, add the following:
percent = ch->pcdata->learned[gsn_bribe]
I'm sure Kayle could give a better explanation other than the fact that you never gave 'percent' a value, which means the code uses NULL, which will crash the mud in this particular instance with the use of bribe, if it ever got past the compile sequence. heh
#4 Jan 12, 2011 7:33 am
Fledgling
GroupMembers
Posts7
JoinedApr 19, 2010
As far as the first warning, like it was said before, the percent is never set to anything before it
is used. Global, static and heap memory that is allocated with calloc will in fact be initialized
to NULL (0), but something on the stack like this will not be set to anything at all. It will be
whatever happens to be in that memory location when the function is entered, and that could
be anything (thus the warning).
In this case, as percent is only used in a math expression, and the result not used for any
range critical use (such as indexing an array), it will not crash the mud. The affect will most
likely be the bribe either always working, or never working as the working range of the check
is quite small as compared with the range an int might be. Though, if it happens to be in the
working range, the function might seem to work just fine (though it could change from call to
call).
As far as what you should do with it, I would rethink setting it to the players bribe skill. You
might as well use:
And get rid of percent, as the skill level will cancel out. Probably percent was intended to be
a random number (maybe 1 to 100).
Ok, now on to the 2nd issue. Now there is a crash waiting to happen (try setting the short
of a mob or race to '%s%s%s%s%s' or the like and see what happens). Note this can be
an issue anytime your code uses user supplied strings as a xprintf format string.
The best fix here as you are just copying the string and doing no formatting is just using:
I have seen quite a number of places in MUD code that uses sprintf (or the like) when a strcpy
would work better.
is used. Global, static and heap memory that is allocated with calloc will in fact be initialized
to NULL (0), but something on the stack like this will not be set to anything at all. It will be
whatever happens to be in that memory location when the function is entered, and that could
be anything (thus the warning).
In this case, as percent is only used in a math expression, and the result not used for any
range critical use (such as indexing an array), it will not crash the mud. The affect will most
likely be the bribe either always working, or never working as the working range of the check
is quite small as compared with the range an int might be. Though, if it happens to be in the
working range, the function might seem to work just fine (though it could change from call to
call).
As far as what you should do with it, I would rethink setting it to the players bribe skill. You
might as well use:
if(victim->top_level > amount)
And get rid of percent, as the skill level will cancel out. Probably percent was intended to be
a random number (maybe 1 to 100).
Ok, now on to the 2nd issue. Now there is a crash waiting to happen (try setting the short
of a mob or race to '%s%s%s%s%s' or the like and see what happens). Note this can be
an issue anytime your code uses user supplied strings as a xprintf format string.
The best fix here as you are just copying the string and doing no formatting is just using:
strcpy(victname, IS_NPC(victim)? victim->short_descr: race_table[victim->race].race_name);
I have seen quite a number of places in MUD code that uses sprintf (or the like) when a strcpy
would work better.
#5 Jan 12, 2011 7:50 am
Magician
GroupMembers
Posts189
JoinedSep 5, 2010
definitely take Sharmair's advice over mine. *LOL* I'm more or less just starting, and trying to be helpful--when maybe I'm not.
#6 Jan 12, 2011 3:50 pm
Last edited Jan 12, 2011 4:17 pm by ayuri
Magician
GroupMembers
Posts239
JoinedJun 13, 2008
Thanks all for the help. Truly appreciate it. I'll poke about and try a few different things. And don't worry Aurin, I'm actually pretty clueless when it comes to programming. I just muddle my way though and yell when I get stuck :D
There are a few more places where I'm seeing uninitialized values. I'll try to poke about and see what I can do now knowing what I'm looking for.
Sharmair,
As to the strcpy/format warnings - I'll also poke them with a stick and see what happens. And thanks for the explanation as to whats going on with the percent warning.
**EDIT
So, as most of you are (or should be) aware that I'm not exactly using FUSS code. As such I've gone back and started to look at some of my warnings and comparing them against the FUSS. Seems I've wasted everyone's time. I'll keep an eye open for an actual bug.
Again, thank you all!
ayuri
There are a few more places where I'm seeing uninitialized values. I'll try to poke about and see what I can do now knowing what I'm looking for.
Sharmair,
As to the strcpy/format warnings - I'll also poke them with a stick and see what happens. And thanks for the explanation as to whats going on with the percent warning.
**EDIT
So, as most of you are (or should be) aware that I'm not exactly using FUSS code. As such I've gone back and started to look at some of my warnings and comparing them against the FUSS. Seems I've wasted everyone's time. I'll keep an eye open for an actual bug.
Again, thank you all!
ayuri
#7 Jan 13, 2011 3:09 am
Magician
GroupMembers
Posts132
JoinedJan 29, 2006
Sharmair said:
I have seen quite a number of places in MUD code that uses sprintf (or the like) when a strcpy would work better.
There's absolutely nothing wrong with using sprintf here, as long as it's invoked correctly (look at the second argument):
sprintf( victname, "%s", IS_NPC( victim ) ? victim->short_descr : race_table[victim->race].race_name );
This is exactly what the compiler's warning says: The call to sprintf lacked the literal string containing format arguments (%s, %d, etc).
Better yet, use snprintf to safeguard against buffer overflow:
snprintf( victname, sizeof(victname), "%s", IS_NPC( victim ) ? victim->short_descr : race_table[victim->race].race_name );
Pages:<< prev 1 next >>