• Anonymous Coward (unregistered)

    I guess changing the format string would have been to simple.

  • (cs)

    String constructs have been around for a while. Can we stop teaching character arrays in language classes/books?

  • Pasty (unregistered) in reply to SurfaceTension

    Hear hear! I am sick of CIS classes that teach language agnostic code in favor of OO concept but fall back to pointless character array massaging that has nothing to do with the implementation of a String object anyway. I have been picking this bone for quite a long time.

  • Anon (unregistered)

    Anding with 0xFF wasn't very necessary either.

  • Eric Smith (unregistered)

    Another reason not to use char arrays:

    char  tempStr[16];
    Is an overflow waiting to happen.  The input could be 18 chars long.

  • (cs)

    The natives must have taken one look at this and went "Ooh. Scary!" then toddled off  and wrote a CS101 implementation of a replace (remove) routine.

  • (cs) in reply to Eric Smith
    Anonymous:

    Another reason not to use char arrays:

    char  tempStr[16];
    Is an overflow waiting to happen.  The input could be 18 chars long.


    It's a one-off function, probably copy-pasted from elsewhere and unlikely to every be called anywhere else... although I'll grant there's a chance one of these code-monkeys could end up calling RemoveColonsFromMacAddr to remove the colons from a path or something.

    Wow, it must have been lovely coming back to find a bunch of twits have hacked in whatever they needed with a crowbar and dynamite.
  • (cs) in reply to Anon
    Anonymous:
    Anding with 0xFF wasn't very necessary either.


    But of course it was necessary! What if they were going to run this code on a computer designed in the 50s or early 60s*, before a 'byte' became universally defined as 8 bits**? Machine independence aside, let's say I did
    this:

    char macstr[18]; // 01:23:45:67:89:AB + \0 terminator

    If one of my bytes was greater than 255, it would overflow my buffer, smash the stack, and someone could take control of an networked machine that may well have almost as much computing power as a barcode scanner!

    * Ignore the fact that these machines preceded Ethernet by at least a couple of decades.
    ** See http://dict.die.net/byte/

  • (cs) in reply to Mike R
    Mike R:
    The natives must have taken one look at this and went "Ooh. Scary!" then toddled off  and wrote a CS101 implementation of a replace (remove) routine.


    I agree. Learning how to use arrays can't hurt, and character arrays can be good for demonstrating array functionality, but aspiring programmers should learn how to use what's already out there (string libraries) before they learn how to circumvent it (character arrays).
  • (cs) in reply to Pasty
    Anonymous:
    Hear hear! I am sick of CIS classes that teach language agnostic code in favor of OO concept but fall back to pointless character array massaging that has nothing to do with the implementation of a String object anyway. I have been picking this bone for quite a long time.


    Well, apparently original code was in C so whoever maintained that code had no choice really.
  • Anonymous (unregistered) in reply to dreifus

    Most of the responses here seem to be a WTF.  The WTF here has nothing to do with the WAY the colons are being removed.  This is C.  IT DOESN"T HAVE STRINGS.  The only way to remove the colons is to do it on a character by character basis.

    The WTF is that the colons are added by the sprintf format string:

     sprintf (macAddrStr, "%02x:%02x:%02x:%02x:%02x:%02x", 

    If you don't need the colons simply remove them from the format string:

    sprintf (macAddrStr, "%02x%02x%02x%02x%02x%02x",

    This whole process specifically adds the colons in, then systematically removes them.




  • (cs)

    Alex Papadimoulis:

    void MacAddrToChar (uint8 *macAddrInt, char *macAddrStr) ;
    void RemoveColonsFromMacAddr (char *macAddrStr) ;

    <FONT face="Courier New" size=2>wow.  this is how it happened:</FONT>

    <FONT face="Courier New" size=2>customer: "our application wants to read in the mac address as a string without colons."
    project manager: "yes, of course.  we'll get on that right away!  steve-dave, the customer wants the format of the mac address changed.  the new requirement is that the mac address string should have no semi-colons.  top priority!  can you get this done by 5:00?"
    steve-dave (looks at watch, reads 4:28): "sure thing!"</FONT>

    <FONT face="Courier New" size=2>meanwhile, inside steve-dave's head:</FONT>

    <FONT face="Courier New" size=2>"hrm.  hexadecimal.  those consultants know their shit!"
    "arg.  there's no way i can figure out this sprintf crap."
    "they're using pointers, too!  this is impossible."
    "4:47!? shit shit shit."
    "ah!  i got it!  i'll just loop through and take out the colons!"
    "there, all done.  check-in...done!  i'm outta here!"</FONT>

    <FONT face="Courier New" size=2>monday morning:</FONT>

    <FONT face="Courier New" size=2>project manager: "laird, we're getting tons of bug reports about the app crashing.  get right on it!"</FONT>

    <FONT face="Courier New" size=2>later that day:</FONT>

    <FONT face="Courier New" size=2>laird: "i can't find the problem.  the system looks hosed.  it's like some kind of fundamental function was changed to do something entirely different."
    project manager: "noooo!  we need to get the consultants here right away!  i just knew they had ripped us off with their buggy system!"</FONT>

    <FONT face="Courier New" size=2>later that week:</FONT>

    <FONT face="Courier New" size=2>consultant: "what the..."</FONT>

  • (cs) in reply to SurfaceTension
    SurfaceTension:
    String constructs have been around for a while. Can we stop teaching character arrays in language classes/books?


    I've had to use character arrays occasionally for input sanitisation...one more level of security to make sure someone wasn't typing "/#hahaha you suck in the password field in hopes of whacking a SQL statement.

    Oooo, scary memory surfaces. I had to work with a TurboImage/XL database for a couple of years once...One of the old flat table jobs, where the "fields" are only defined in terms of numbers of characters, so you know that the "name" field starts at position 23 and ends at position 62, and every line of data fetched had to be split into it's components to be used. Talk about a nightmare.

    No doubt some of those things are still around, so it's probably not time to kill characters just yet.
  • (cs) in reply to SurfaceTension

    There are many good reasons to use a character array, size and speed being the two that immediately come to mind.  For my current application, my code needs to compile to be as small as possible.  Objects such as std::string or CString take a considerable amount of behind the scenes code to manage, compared to just using a built-in type.  Besides the cost of actually using the library (STL Port's version of std::string adds something like 1.5K into your executable), the code associated with creating and destroying objects is considerable.  I'm only talking about 40 or so bytes in this example, but 40 bytes extra in 1000 places would more than double the size of some of my executables.

    The following code is considerably smaller (less than half the size with Dev Studio 6):

    char *bob = new bob(strlen(bobA)+strlen(bobB)+strlen(bobC)+1);
    strcpy(bob,bobA);
    strcat(bob,bobB);
    strcat(bob,bobC);
    ...
    delete [] bob;  //This is only bad if you need to throw exceptions or return in many places


    than if you use an object:

    std::string bob(bobA);
    bob += bobB;
    bob += bobC;


    which is smaller than:
    std::string bob = bobA + bobB + bobC;  //This creates a temporary std::string to assign to bob



    I'm guessing size was not a consideration in the code above though.  Clearly they could have made their code smaller by removing tempStr completely and doing an inplace move in macAddrStr :

    void RemoveColonsFromMacAddr (char *macAddrStr)
    {
    int i, j;

    j = 0;
    for (i = 0; i < strlen (macAddrStr); i++)
    {
    if (macAddrStr[i] != ':')
    {
    macAddrStr[j] = macAddrStr[i];
    j++;
    }
    }
    macAddrStr[j] = '\0';
    }

  • Anonymous (unregistered) in reply to SurfaceTension
    SurfaceTension:
    String constructs have been around for a while. Can we stop teaching character arrays in language classes/books?


    Sure.  Just as soon as we stop teaching low level languages that don't HAVE strings.
  • (cs) in reply to Anon
    Anonymous:
    Anding with 0xFF wasn't very necessary either.


    Are you sure?  I think each of those arguments are passed to sprintf as ints.
  • Dave (unregistered) in reply to Anon

    Depends whether the utf8 type is signed or not.

  • (cs)

    I'm wondering if all the code presented to the public is written by internal developers.

  • (cs) in reply to SurfaceTension

    SurfaceTension:
    String constructs have been around for a while. Can we stop teaching character arrays in language classes/books?

    As long as there is hardware which does not directly implement string constructs, we will need character arrays.

     

  • (cs) in reply to Dave
    Anonymous:
    Depends whether the utf8 type is signed or not.


    Well, it's not utf8, it's uint8.  Which conventionally mean unsigned 8-bit int...  Which is usually an unsigned char.  And if that's the case, then it works...  but not because it's unsigned, but because it's 8 bits and passed by value.

    But the fact that I had to spend 10 minutes figuring it out means that putting the 0xFF is worthwhile, since it removes all doubt.
  • (cs) in reply to Oliver Klozoff
    Oliver Klozoff:
    But of course it was necessary! What if they were going to run this code on a computer designed in the 50s or early 60s*, before a 'byte' became universally defined as 8 bits**?


    Then the code wouldn't compile, because there's no way to get an uint8 type on that platform. Simple.
  • Jan (unregistered) in reply to Luffy

    Luffy:

    I'm guessing size was not a consideration in the code above though.  Clearly they could have made their code smaller by removing tempStr completely and doing an inplace move in macAddrStr :

    void RemoveColonsFromMacAddr (char *macAddrStr)
    {
    int i, j;

    j = 0;
    for (i = 0; i < strlen (macAddrStr); i++)
    {
    if (macAddrStr[i] != ':')
    {
    macAddrStr[j] = macAddrStr[i];
    j++;
    }
    }
    macAddrStr[j] = '\0';
    }


    You've missed the point: you're optimizing code that shouldn't be there in the first place.

  • Rambo (unregistered) in reply to Jan

    I just wonder where all these horrible programmers come from.

  • (cs) in reply to Luffy
    Luffy:


    char *bob = new bob(strlen(bobA)+strlen(bobB)+strlen(bobC)+1);
    strcpy(bob,bobA);
    strcat(bob,bobB);
    strcat(bob,bobC);
    ...
    delete [] bob;  //This is only bad if you need to throw exceptions or return in many places


    Here is a better idea that is exception safe:

    char *bob = (char *)alloca(strlen(bobA)+strlen(bobB)+strlen(bobC)+1);

    If you want to make it fast you could:
    size_t alen = strlen(bobA);
    size_t blen = strlen(bobB);
    size_t clen = strlen(bobC);
    char *bob = (char *)alloca(alen + blen + clen + 1);
    strcpy(bob, bobA);
    strcpy(&bob[alen], bobB);
    strcpy(&bob[alen + blen], bobC);
    bob[alen+blen+clen] = '\0';

    Exception safe, faster than any string class, and cannot leak memory.   Course the first non-expert programmer to come along will not understand it, go WTF, and introduce a lot of bugs.   (Actually since I didn't test this I wouldn't be surprised if I wrote a bug in there, even though I've done essentially this more than once)

    If you want to do it right:
    size_t boblen = strlen(bobA)+strlen(bobB)+strlen(bobC)+1
    char *bob = (char *)alloca(boblen);
    strlcpy(bob,bobA,boblen);
    strlcat(bob,bobB,boblen);
    strlcat(bob,bobC,boblen);

    No bugs (other than maybe a syntax error the compiler would catch), hard to mess up.   Works are all modern operation systems except Linux because the glibc guys are convinced that real programmers never make a mistake in C strings, so they won't give you the safe strl* functions for handling strings.

    
    
    
    
    
    
    As you might guess, I've had to work with C pointers and
    

    character arrays an awful lot in my "C++" code.   (Actually much of the time I'm working with void *, and treating it like a sequence of bytes.   Embedded nulls are expected.   I've done it with strings too though because out lead programmer hates the STL enough to forbid using it, but that is a different rant)


  • (cs) in reply to Eric Smith
    Anonymous:

    Another reason not to use char arrays:

    char  tempStr[16];
    Is an overflow waiting to happen.  The input could be 18 chars long.



    Um, no, it will ALWAYS be 13 characters long, including the '\0' at the end.  Always.

    But tempStr is completely unnecessary - this works just fine:

    void RemoveColonsFromString(char *m)
    {

        for (char *p = m; *m != '\0'; m++)
        {
            if (*m != ':')
            {
                *p++ = *m;
            }
        }

        *p = '\0';

        return;
    }

  • (cs)

    Everybody seems to have missed the most obvious replacement for the original chunk of code, so we might as well include it for completeness sake.

    void MacAddrToChar (uint8 *macAddrInt, char *macAddrStr)
    {
      sprintf (macAddrStr, "%12x", (*macAddrInt));
    }
  • Arachnid (unregistered) in reply to Otto
    Otto:

    Everybody seems to have missed the most obvious replacement for the original chunk of code, so we might as well include it for completeness sake.

    void MacAddrToChar (uint8 *macAddrInt, char *macAddrStr)
    {
      sprintf (macAddrStr, "%12x", (*macAddrInt));
    }


    That'll return the hexadecimal value of the location of the pointer to the MAC address, not the actual MAC address, or even its location.
  • Arachnid (unregistered) in reply to Arachnid
    Anonymous:
    Otto:

    Everybody seems to have missed the most obvious replacement for the original chunk of code, so we might as well include it for completeness sake.

    void MacAddrToChar (uint8 *macAddrInt, char *macAddrStr)
    {
      sprintf (macAddrStr, "%12x", (*macAddrInt));
    }


    That'll return the hexadecimal value of the location of the pointer to the MAC address, not the actual MAC address, or even its location.


    Oops. Correction: That'll return a 12 digit hexadecimal string of the first digit in the MAC. Eg, 0000000000xx.
  • David P. Murphy (unregistered) in reply to Otto
    Otto:

    Everybody seems to have missed the most obvious replacement for the original chunk of code, so we might as well include it for completeness sake.

    void MacAddrToChar (uint8 *macAddrInt, char *macAddrStr)
    {
      sprintf (macAddrStr, "%12x", (*macAddrInt));
    }


    The most obvious wrong replacement, since the third argument to sprintf() will be an unsigned char promoted to an int.
    In other words, only one-sixth of the intended value.

    ok
    dpm
  • (cs) in reply to Otto
    Otto:

    SurfaceTension:
    String constructs have been around for a while. Can we stop teaching character arrays in language classes/books?

    As long as there is hardware which does not directly implement string constructs, we will need character arrays.

    I'm not sure what you are getting at here.  'String' is a programming abstraction as is 'character array'.  What does that have to do with hardware?

  • David P. Murphy (unregistered) in reply to Maurits
    Maurits:


    Um, no, it will ALWAYS be 13 characters long, including the '\0' at the end.  Always.



    Your reason for assuming that RemoveColonsFromString()

    will never be called from any other function is not apparent to me.

    Do you make this mistake in real life?


    ok

    dpm

  • Eric Smith (unregistered) in reply to Maurits

    No, it will always be 18 characters long, including the '\0' at the end.  You forgot about the colons!  6 * 2  (for the hex digits) + 5 * 1 (for the colons) + 1 = 18.

  • (cs) in reply to hank miller
    hank miller:
    No bugs (other than maybe a syntax error the compiler would catch), hard to mess up.   Works are all modern operation systems except Linux because the glibc guys are convinced that real programmers never make a mistake in C strings, so they won't give you the safe strl* functions for handling strings.
    
    
    
    
    
    
    As you might guess, I've had to work with C pointers and
    

    character arrays an awful lot in my "C++" code.   (Actually much of the time I'm working with void *, and treating it like a sequence of bytes.   Embedded nulls are expected.   I've done it with strings too though because out lead programmer hates the STL enough to forbid using it, but that is a different rant)


    Be nice if borland included it in older C++ compiler versions. Since it didn't, I just put it in my project:
    http://fxr.watson.org/fxr/source/libkern/strlcpy.c
    http://fxr.watson.org/fxr/source/libkern/strlcat.c

    They shouldn't add much to the size of any C program; my own rewrites are a barbaric combination of these and the optimizations of the regular strcat/spy, but that's not your goal.

  • (cs) in reply to Eric Smith
    Anonymous:
    No, it will always be 18 characters long, including the '\0' at the end.  You forgot about the colons!  6 * 2  (for the hex digits) + 5 * 1 (for the colons) + 1 = 18.

    I pray I never have you verifying my code.
  • (cs) in reply to hank miller
    hank miller:

    Here is a better idea that is exception safe:

    char *bob = (char *)alloca(strlen(bobA)+strlen(bobB)+strlen(bobC)+1);

    If you want to make it fast you could:
    size_t alen = strlen(bobA);
    size_t blen = strlen(bobB);
    size_t clen = strlen(bobC);
    char *bob = (char *)alloca(alen + blen + clen + 1);
    strcpy(bob, bobA);
    strcpy(&bob[alen], bobB);
    strcpy(&bob[alen + blen], bobC);
    bob[alen+blen+clen] = '\0';

    Exception safe, faster than any string class, and cannot leak memory.

    Thank you for the suggestion Hank.  I'm sure that code is faster, but unfortunately this code is actually 32 bytes bigger.  But is the same size, and exception safe, if you simply use alloca instead of new:

    char *bob = (char*)alloca(strlen(bobA)+strlen(bobB)+strlen(bobC)+1);
    strcpy(bob,bobA);
    strcat(bob,bobB);
    strcat(bob,bobC);

    Use of alloca or new depends on available stack space.  alloca definitely uses less memory than an tr1::shared_ptr!!

    Sadly my compiler does not support strlcpy and strlcat.

    jan:

    You've missed the point: you're optimizing code that shouldn't be there in the first place.


    Thanks Jan, you made my day.  ^_^  I was hoping someone would enjoy my pointless "correction".
  • (cs) in reply to Eric Smith
    Anonymous:
    No, it will always be 18 characters long, including the '\0' at the end.  You forgot about the colons!  6 * 2  (for the hex digits) + 5 * 1 (for the colons) + 1 = 18.


    The input will, sure.  But the temp working area doesn't need to be as big as the input.  18 is better than 16, but 13 is better than both of them.

    But your point is taken.  Why introduce an artificial limit?  Better to measure the length of the input string and do a malloc/free, or avoid the issue altogether by doing an in-place replacement.

    To answer someone else's question, I assume that the function will only be called with MAC addresses, because of the way its named.  But there's a good point there, too... MAC addresses aren't always separated with colons, sometimes people use dashes.  So perhaps the if (...) should be
    if (
        ('0' <= *m && *m <= '9') ||
        ('a' <= *m && *m <= 'f') ||
        ('A' <= *m && *m <= 'F')
    )
    {
        /* extract *m ... */
    }
  • Grant (unregistered) in reply to Maurits
    Maurits:
    Anonymous:

    Another reason not to use char arrays:

    char  tempStr[16];
    Is an overflow waiting to happen.  The input could be 18 chars long.



    Um, no, it will ALWAYS be 13 characters long, including the '\0' at the end.  Always.

     

    RemoveColonsFromString("AreYou100PercentAbsolutelyPositivelySureAboutThat?");

  • (cs) in reply to David P. Murphy
    Anonymous:

    Your reason for assuming that RemoveColonsFromString()

    will never be called from any other function is not apparent to me.



    Um, the function's name is RemoveColonsFromMacAddr( )
    It's fair to assume that the input will be a MAC address, delimited either by colons or by nothing at all.

    Anonymous:

    Do you make this mistake in real life?


    No, in real life I, too, try to avoid arbitrary limits.  See my sample code.
  • (cs) in reply to David P. Murphy
    Anonymous:
    Do you make this mistake in real life?


    I do make other mistakes, though. :)  Here's a version of RemoveColonsFromString(char *s) that actually compiles and runs and works and everything :)

    void RemoveColonsFromString(char *s)
    {
        char *t;

        for (t = s; *s != '\0'; s++)
        {
            if (*s != ':')
            {
                    if (t != s && *t != *s)
                    {
                            *t = *s;
                    }
                    t++;
            }
        }

        if (*t != '\0') { *t = '\0'; }

        return;
    }

  • (cs)

    Okay, I'll try to defend this one...

    I imagine that the Mac address will be needed both with colons (as probably required by the original code base), and without colons: probably the result of some new requirements and new additions to the code.

    In this case, what are the solutions?

    1. Write a new function, MacAddrToCharWithoutColons, that is exactly the same as the previous one, but without the colons in the format string. This leads to code duplication.
    2. Just "take the colons out" if you don't need them (declaring a separate function if needed). This sounds most logical and is the approach I would take as well.

    Now, because they are using C, the second method is actually more cumbersome and less readable than the first one, but I can appreciate where these guys (probably) came from: a language that gives you a string-replace function.

  • Jon (unregistered) in reply to hank miller
    hank miller:
    Here is a better idea that is exception safe:

    char *bob = (char *)alloca(strlen(bobA)+strlen(bobB)+strlen(bobC)+1);
    As I understand it, alloca() is a non-standard function in C++, and has been implemented in non-exception-safe ways in the past.
  • The letter N (unregistered) in reply to RiX0R
    RiX0R:
    I imagine that the Mac address will be needed both with colons (as probably required by the original code base), and without colons: probably the result of some new requirements and new additions to the code.


    Except MacAddrToChar itself calls RemoveColonsFromMacAddr.
  • Arachnid (unregistered) in reply to Maurits
    Maurits:
    Anonymous:
    Do you make this mistake in real life?


    I do make other mistakes, though. :)  Here's a version of RemoveColonsFromString(char *s) that actually compiles and runs and works and everything :)

    void RemoveColonsFromString(char *s)
    {
        char *t;

        for (t = s; *s != '\0'; s++)
        {
            if (*s != ':')
            {
                    if (t != s && *t != *s)
                    {
                            *t = *s;
                    }
                    t++;
            }
        }

        if (*t != '\0') { *t = '\0'; }

        return;
    }



    Now to take it back in the opposite direction: obfuscation (why? Because I'm bored.).

    void RemoveColonsFromString(char *s, char *t) {
      *s-58?0[t++]=0[s++]:s++;
      0[s-1]?RemoveColonsFromString(s, t):t;
    }

    Can anyone make it more obfuscated?
  • Ran (unregistered) in reply to RiX0R
    RiX0R:

    Okay, I'll try to defend this one...

    I imagine that the Mac address will be needed both with colons (as probably required by the original code base), and without colons: probably the result of some new requirements and new additions to the code.

    In this case, what are the solutions?

    1. Write a new function, MacAddrToCharWithoutColons, that is exactly the same as the previous one, but without the colons in the format string. This leads to code duplication.
    2. Just "take the colons out" if you don't need them (declaring a separate function if needed). This sounds most logical and is the approach I would take as well.

    Now, because they are using C, the second method is actually more cumbersome and less readable than the first one, but I can appreciate where these guys (probably) came from: a language that gives you a string-replace function.



    Not really. I mean, that would make some sort of sense, but it can't be the explanation here, as they modified MacAddrToChar to call their take-the-colons-out function; so they no longer have a version *with* colons.
  • (cs) in reply to Arachnid
    Anonymous:


    Now to take it back in the opposite direction: obfuscation (why? Because I'm bored.).

    void RemoveColonsFromString(char *s, char *t) {
      *s-58?0[t++]=0[s++]:s++;
      0[s-1]?RemoveColonsFromString(s, t):t;
    }

    Can anyone make it more obfuscated?


    ? Doesn't compile for me (gcc)
    warning: pointer/integer type mismatch in conditional expression
    I get that ':' == 58, but what does 0[t] do?
  • Arachnid (unregistered) in reply to Maurits
    Maurits:
    Anonymous:


    Now to take it back in the opposite direction: obfuscation (why? Because I'm bored.).

    void RemoveColonsFromString(char *s, char *t) {
      *s-58?0[t++]=0[s++]:s++;
      0[s-1]?RemoveColonsFromString(s, t):t;
    }

    Can anyone make it more obfuscated?


    ? Doesn't compile for me (gcc)
    warning: pointer/integer type mismatch in conditional expression
    I get that ':' == 58, but what does 0[t] do?


    It gives a warning, but it still compiles just fine.

    x[y] is exactly equivalent to y[x] is exactly equivalent to *(x + y). It's all just syntactic sugar. :)  
  • (cs) in reply to Arachnid
    Anonymous:

    It gives a warning, but it still compiles just fine.

    x[y] is exactly equivalent to y[x] is exactly equivalent to *(x + y). It's all just syntactic sugar. :)  


    Ah, I see... yes, beautiful :)
  • David P. Murphy (unregistered) in reply to Maurits
    Maurits:
    David P. Murphy:

    Your reason for assuming that RemoveColonsFromString()

    will never be called from any other function is not apparent to me.



    Um, the function's name is RemoveColonsFromMacAddr( )
    It's fair to assume that the input will be a MAC address, delimited either by colons or by nothing at all.



    That's it?   That's your reason?

    I fail to see how the name of the function will prevent it from being called improperly.
    Someday there will be an access violation, and your "reason" is not going to impress anyone.

    Just admit that the code sucks in many ways, and this is one of them.

    ok
    dpm
  • (cs) in reply to David P. Murphy
    Anonymous:
    Maurits:
    David P. Murphy:

    Your reason for assuming that RemoveColonsFromString()

    will never be called from any other function is not apparent to me.



    Um, the function's name is RemoveColonsFromMacAddr( )
    It's fair to assume that the input will be a MAC address, delimited either by colons or by nothing at all.



    That's it?   That's your reason?

    I fail to see how the name of the function will prevent it from being called improperly.
    Someday there will be an access violation, and your "reason" is not going to impress anyone.

    Just admit that the code sucks in many ways, and this is one of them.

    ok
    dpm


    Well, you should still remove the pointless add-colons-then-remove-them from MacAddrToChar(), whether you keep RemoveColonsFromMacAddr() around or not.

  • suidae (unregistered) in reply to hank miller
    hank miller:

    size_t alen = strlen(bobA);
    size_t blen = strlen(bobB);
    size_t clen = strlen(bobC);
    char *bob = (char *)alloca(alen + blen + clen + 1);
    strcpy(bob, bobA);
    strcpy(&bob[alen], bobB);
    strcpy(&bob[alen + blen], bobC);
    bob[alen+blen+clen] = '\0';

    Exception safe, faster than any string class, and cannot leak memory.   Course the first non-expert programmer to come along will not understand it, go WTF, and introduce a lot of bugs.


    Well, sure, but only because some guy who likes to abuse the people who have to maintain projects didn't bother to put in comments explaining what the code is intended to do, and why it does it that way.  At the very least, he could have included a '//WARNING COMPLEX CODE AHEAD'

    Seriously, when writing code that someone will have to maintain (often some less experianced than the original author) its a good idea to explain intension, unusual methods or finer points that they might not see right away.  There are a lot of new guys out there that get stuck on maintainance, show 'em why you do what you do so they can learn faster.

Leave a comment on “Colonitis”

Log In or post as a guest

Replying to comment #:

« Return to Article