Author Topic: Unsequenced modification and access to 'n'  (Read 1832 times)

Offline QwazyWabbit

  • Carpal Tunnel Member
  • ******
  • Posts: 1243
    • View Profile
  • Rated:
Unsequenced modification and access to 'n'
« on: May 12, 2016, 01:28:14 AM »
While hacking TMG mod I found this little gem:

Code: [Select]
// From QDevels, posted by outlaw
void convert_string(char *src, char start, char end, char add, char *dest)
{
    int n = -1;
    while ((dest[++n] = src[n]))
        if ((dest[n] >= start) && (dest[n] <= end) && (dest[n] != '\n'))
            dest[n] += add;
}

/* Examples of convert_string usage
 {
 char  text[] = "abcdefgABCDEFG1234567\n\0";
 convert_string(text, 'a', 'z', ('A'-'a'), text); // a -> A
my_bprintf (PRINT_CHAT, "text = %s\n", text);
 convert_string(text, 'A', 'Z', ('a'-'A'), text); // A -> a
my_bprintf (PRINT_CHAT, "text = %s\n", text);
 convert_string(text, 0, 127, 128, text); // white -> green
my_bprintf (PRINT_CHAT, "text = %s\n", text);
 convert_string(text, 128, 255, -128, text); // green -> white
my_bprintf (PRINT_CHAT, "text = %s\n", text);
 }
 */

What first caught my eye was the warning from the compiler per the subject line but second was the fun and exciting int n = -1; initialization. OK, so we're being really 1337 coder and were starting the index off below the array bounds of dest and the ++n is going to make it 0 before it puts the value of src[n] into the destination.... or does it?

The warning is about that dest[++n] thing. This is a reliance on undefined behavior in C and it can be a nasty bug. It will work fine in one platform and fail in another and it can take some time to fix if it's buried deep enough. As it was, Microsoft Visual Studio 2010 likes it and it works. It also works on Ubuntu and GCC 4.8.4 but it doesn't work on OS X, using clang/LLVM. Since Microsoft is migrating to using clang on their compiler suite you can expect this function will break when compiled on the new tools or in their Azure cloud. It may also fail on newer versions of GCC on other Linux distributions.

OK, so how to fix it? Answer: pointers instead of array indexes.

Code: [Select]
/**
 Replace characters in destination string.
 Parameter 'add' is added to each character found in source and result is placed in dest.
 Parameters 'start' and 'end' specify character range to replace.
 Source text must be a valid C string.
*/
//QwazyWabbit// A pointer version to eliminate undefined behavior.
void convert_string(char *src, char start, char end, char add, char *dest)
{
while ((*dest = *src)) {
if ((*dest >= start) && (*dest <= end) && (*dest != '\n'))
*dest += add;
src++, dest++;
}
}

In keeping with the existing 1337ness of the code, I made good use of the comma operator. :)

If your projects contain this original Qdevels/outlaw function I urge you replace it ASAP. You'll know the old one is broken when your Q2 strings and chats suddenly become blank when you port your mod instead of green or white.

In addition, I offer the following wrapper functions to use in place of the rather complicated direct uses of the 'convert_string' function:

Code: [Select]
/**
 Set msb in specified string characters, copying them to destination.
 Text must be a valid C string.
 Source and destination can be the same.
 If dest == NULL the action occurs in-place.
 */
void highlight_text (char *src, char *dest)
{
if (dest == NULL)
dest = src;
convert_string(src, 0, 127, 128, dest); // white -> green
}

/**
 Clear msb in specified string characters, copying them to destination.
 Text must be a valid C string.
 Source and destination can be the same.
 If dest == NULL the action occurs in-place.
 */
void white_text (char *src, char *dest)
{
if (dest == NULL)
dest = src;
convert_string(src, 128, 255, -128, dest); // green -> white
}

/**
 Make text uppercase.
 Text must be a valid C string.
 Source and destination can be the same.
 If dest == NULL the action occurs in-place.
 */
void toupper_text(char *src, char *dest)
{
if (dest == NULL)
dest = src;
convert_string(src, 'a', 'z', ('A'-'a'), dest); // a -> A
}

/**
 Make text lowercase.
 Text must be a valid C string.
 Source and destination can be the same string.
 If dest == NULL the action occurs in-place.
 */
void tolower_text(char *src, char *dest)
{
if (dest == NULL)
dest = src;
convert_string(src, 'A', 'Z', ('a'-'A'), dest); // A -> a
}


Testbed code:

Code: [Select]
#include <stdio.h>

int main(int argc, const char * argv[])
{
char otext[] = "abcdefgABCDEFG1234567";
char text[]  = "abcdefgABCDEFG1234567\n";
char target[sizeof text];

toupper_text(otext, target); // a -> A
printf ("otext   = %s\n", otext);
printf ("target  = %s\n", target);
toupper_text(text, NULL); // a -> A
printf ("text    = %s\n", text);

tolower_text(otext, target); // A -> a
printf ("otext   = %s\n", otext);
printf ("target  = %s\n", target);
tolower_text(text, NULL); // A -> a
printf ("text    = %s\n", text);

highlight_text(otext, target); // white -> green
printf ("otext   = %s\n", otext);
printf ("target  = %s\n", target);
highlight_text(text, NULL); // white -> green
printf ("text    = %s\n", text);

white_text(target, otext); // green -> white
printf ("target  = %s\n", target);
printf ("otext   = %s\n", otext);
white_text(text, NULL); // green -> white
printf ("text    = %s\n", text);

return 0;
}
  • Insightful
    Informative
    Funny
    Nice Job / Good Work
    Rock On
    Flawless Logic
    Well-Reasoned Argument and/or Conclusion
    Demonstrates Exceptional Knowlege of the Game
    Appears Not to Comprehend Game Fundamentals
    Frag of the Week
    Frag Hall of Fame
    Jump of the Week
    Jump Hall of Fame
    Best Solution
    Wins The Internet
    Whoosh! You done missed the joke thar Cletus!
    Obvious Troll Is Obvious
    DO YOU EVEN LIFT?
    DEMO OR STFU
    Offtopic
    Flamebait
    Redundant
    Factually Challenged
    Preposterously Irrational Arguments
    Blindingly Obvious Logical Fallacies
    Absurd Misconstrual of Scientific Principles or Evidence
    Amazing Conspiracy Theory Bro
    Racist Ignoramus

Offline beaver{KEA}

  • Sr. Member
  • ****
  • Posts: 323
  • Join Quake Legacy Mod Revival Discord!
    • View Profile
    • Mod Revival made easy on discord chat!
  • Rated:
Re: Unsequenced modification and access to 'n'
« Reply #1 on: May 13, 2016, 08:40:18 PM »
i had a friend try setting up tmg on frebsd and he had that issue with the text. no players names etc.
  • Insightful
    Informative
    Funny
    Nice Job / Good Work
    Rock On
    Flawless Logic
    Well-Reasoned Argument and/or Conclusion
    Demonstrates Exceptional Knowlege of the Game
    Appears Not to Comprehend Game Fundamentals
    Frag of the Week
    Frag Hall of Fame
    Jump of the Week
    Jump Hall of Fame
    Best Solution
    Wins The Internet
    Whoosh! You done missed the joke thar Cletus!
    Obvious Troll Is Obvious
    DO YOU EVEN LIFT?
    DEMO OR STFU
    Offtopic
    Flamebait
    Redundant
    Factually Challenged
    Preposterously Irrational Arguments
    Blindingly Obvious Logical Fallacies
    Absurd Misconstrual of Scientific Principles or Evidence
    Amazing Conspiracy Theory Bro
    Racist Ignoramus
Bring more Life back to your favorite mod! on discord chat!#gloom #vortex #qpong #kots #giex #q2jump #ra2 #wod #wodx #railwarz #LMCTF #LFireCTF #expertctf https://discord.gg/44gk8j9

Offline QwazyWabbit

  • Carpal Tunnel Member
  • ******
  • Posts: 1243
    • View Profile
  • Rated:
Re: Unsequenced modification and access to 'n'
« Reply #2 on: May 13, 2016, 09:42:17 PM »
That's very interesting, OS X is based on FreeBSD. The compiler version will have the most influence of course. What version compiler does he have and what version OS?

The convert_string function is in g_utils.c in the TMG mod. You can copy-paste the code from here into your copy. The archive I posted May 7 has the bug, the one I posted May 8 has a version of my replacement and should work correctly.
  • Insightful
    Informative
    Funny
    Nice Job / Good Work
    Rock On
    Flawless Logic
    Well-Reasoned Argument and/or Conclusion
    Demonstrates Exceptional Knowlege of the Game
    Appears Not to Comprehend Game Fundamentals
    Frag of the Week
    Frag Hall of Fame
    Jump of the Week
    Jump Hall of Fame
    Best Solution
    Wins The Internet
    Whoosh! You done missed the joke thar Cletus!
    Obvious Troll Is Obvious
    DO YOU EVEN LIFT?
    DEMO OR STFU
    Offtopic
    Flamebait
    Redundant
    Factually Challenged
    Preposterously Irrational Arguments
    Blindingly Obvious Logical Fallacies
    Absurd Misconstrual of Scientific Principles or Evidence
    Amazing Conspiracy Theory Bro
    Racist Ignoramus

 

El Box de Shoutamente

Last 10 Shouts:

-Unh0ly-

December 03, 2019, 09:19:59 PM
NEW ! texture pak  V 2.0 get it at http://unh0ly.wordpress.com

Some screen shots on website.
 

quadz

November 29, 2019, 11:00:53 AM
Some snazzy updates to the ray-traced version of Q2:


https://www.youtube.com/watch?v=CnYtathGvdw
 

Prophet

November 28, 2019, 10:10:38 AM
Did the server just crash? Dont see the address pulling up all of a sudden after I got a phonejack
 

|iR|Focalor

November 27, 2019, 11:55:59 PM
 

Admin

November 26, 2019, 07:22:56 PM
Hm. And the network had seemed so well-behaved of late.
 

|iR|Focalor

November 26, 2019, 12:23:48 PM
Servers went haywire again. Was in the middle of a game on vanilla when the graph started showing green sawtooth stuff. My address book is only showing IRTDM1, IRTDM2 and FOX. No DM, no Vanilla, no Railz.
 

quadz

November 24, 2019, 10:14:15 PM
 

quadz

November 22, 2019, 07:38:43 PM
FYI - massive data breach

https://www.dataviper.io/blog/2019/pdl-data-exposure-billion-people/

"1.2 billion unique people, and 650 million unique email addresses"

RyU

November 14, 2019, 04:31:34 AM
since 06 bro no I dont cheat  :)

-Unh0ly-

November 13, 2019, 10:18:16 AM
uhhhh q2 servers .com is down

Show 50 latest
Welcome, Guest. Please login or register.
December 07, 2019, 04:07:37 PM

Login with username, password and session length