• (unregistered)

    <font color="#0000ff">goto</font>. What more is there to say.

  • (unregistered) in reply to

    Just to add, I especially like the thought, effort and completeness used to accomplish:

        <font color="#0000ff">goto</font> H2A_ACCEPT;
    H2A_ACCEPT:

    Congratulations.


  • (unregistered) in reply to

    I will have nightmares, and in those nightmares I will see that goto keyword...

  • (unregistered)

    If you are criticizing this code, then you obviously dont have an understanding of pointers.  Also, for those who will criticize the use of the goto statement, it is perfectly valid to use it.  This code would not be as easy to read without the use of goto. 

    Actually, we need a function like this where I am working now, does anyone know if the code posted on this site is copyrighted?  Can I use it on my project?

     

  • (cs)

    Does "illusive" mean it makes me ill?  [+o(]  

  • (unregistered)

    This is better than most of the WTFs here, though I'm not I'd use gotos in such a simple and short algorithm (going to a regular while/if syntax might make it a little longer but a whole lot easier to follow).

    The biggest problem I see, if I'm right, is that the strtol( from - 2, NULL, 16 ) call can possibly return an incorrect value. For example the string "%02A" would result in the ASCII string "*A" instead of " A" (with the leading char 0x02).

  • (cs)

    Someone really dislikes for-next loops in here...

  • (cs)

    Should have fired up WSCRIPT in interactive mode and used the VB hex/asc functions.

  • (cs)

    What exactly are you objecting to here? This is just a classic state machine. How else would you implement a state machine?

  • (cs) in reply to stark

    Right, got one. Abuse of ! operator for testing whether a value is zero.

    I only ever use ! for testing a boolean variable, i.e. bool or BOOL. In all other circumstances - both pointers and integers - I use the comparison operators == or != explicitly.

    So, all of those !size or !--size I would rewrite as if( size == 0 ) or if ( --size == 0 ). I also wouldn't combine the decrement operator with the test, I'd make it a separate statement.

  • (cs) in reply to Mike Dimmick

    (Darn, can't edit.)

    Otherwise I agree, it's a fairly canonical one-character lookahead finite state machine. Admittedly I'd probably use flex or a similar tool to generate the FSM for me. I'd also change the label/goto nonsense to a switch/case statement wrapped in a while (!accepted) loop.

    Actually I didn't notice at first that it wasn't. I guess that's a case of seeing what you want, or expect, to see.

  • (cs) in reply to stark

    stark:
    What exactly are you objecting to here? This is just a classic state machine. How else would you implement a state machine?

    2 words: Spaghetti Code.

     

  • (cs) in reply to Mike Dimmick

    Mike Dimmick:
    So, all of those !size or !--size I would rewrite as if( size == 0 ) or if ( --size == 0 ). I also wouldn't combine the decrement operator with the test, I'd make it a separate statement.

    Glad I'm learning C++ for school right now else I'd believe you're right. But no... In C++ a boolean is either zero or non-zero, no matter what type it is. So (!size) is still correct usage of the NOT operator. Yes, it seems funny but C++ is a funny language.

    It is definitely not abuse of the ! operator because the C++ standard declares this as legal usage.

     

    So you're wrong! Nananaanaaaana! [:P]

  • (unregistered)

    Hmm? Using ! to test for zero, and using ++/-- is perfectly normal in C. And your really shouldn't need a state machine for such a simple algorithm...

    What nobody has noticed yet is that this code doesn't actually work: strtol is used incorrectly. Look at what this fragment would do with "%1234"...

  • (cs) in reply to Katja

    I don't think he was saying you can't use the NOT operator in this situation. !size is perfectly valid C code and will compile. But if you're coming to this code for the first time, which is easier to read, (!size) or (size == 0)?

    But the biggest WTF is all those gotos. Until this very moment, I had no idea C/C++ even HAD a goto operator. All these years I assumed it was just a VB monstrosity.

     

  • (unregistered)

    This isn't excessively horrible for C code.  But it serves as a warning to anyone hiring a C++ coder.  Beware of guys like this.

    Your "C coder at heart" guy will insert this kind of bug prone code into your C++ codebase rather than using the approprate collection class if you don't keep microscope on them at all times. 

    People who started with C and then latter pick up a little C++ are considered dangerous.

  • (cs) in reply to spooky action at a distance

    Spooky,

    As you can see it's very real, but I'm sure you can understand why this dark and arcane secret is rarely taught.

    In the assembly I was using for a motorolla processor, an equivalent goto was required for any loop you created (eek!). Good thing most of us don't need to use asm d:

  • (cs) in reply to Katja
    Katja:

    Glad I'm learning C++ for school right now else I'd believe you're right. But no... In C++ a boolean is either zero or non-zero, no matter what type it is. So (!size) is still correct usage of the NOT operator. Yes, it seems funny but C++ is a funny language.

    It is definitely not abuse of the ! operator because the C++ standard declares this as legal usage.

    Just because something compiles doesn't mean it's right.   C (and therefore C++, due to backward compatibility), will allow you to get away with lots of ugliness --- but that doesn't mean that you should do it. 

    I think the worst use of it is in a line like:

    <FONT face="Courier New">if (!strcpy(str,"ABC"))</FONT>

    Now, I think most people when seeing that, would at first read it as "If str is NOT equal to 'ABC' " (when it in fact means "If str IS equal to 'ABC' ")

    Using ! on non-bools is a bad practice --- Remember, if someone is too lazy to type TWO EXTRA CHARACTERS, just think about what other "shortcuts" they might have done....

  • (unregistered)

    2 words: Java Script.

    Okay, that's one word. But you get my meaning.

  • (cs) in reply to JamesCurran
    JamesCurran:

    <FONT face="Courier New">if (!strcpy(str,"ABC"))</FONT>

    Now, I think most people when seeing that, would at first read it as "If str is NOT equal to 'ABC' " (when it in fact means "If str IS equal to 'ABC' ")

    I'd rather say that's the most harmless instance you could pick. That is, could you have bothered to use the right function, strcmp. Everybody knows this function, and, although you don't really seem to know what "compare" means as opposed to "equals", you know exactly what !strcmp(str, "whatever") means...
  • (cs) in reply to stark

    stark:
    What exactly are you objecting to here? This is just a classic state machine. How else would you implement a state machine?

    Besides the fact that you don't need a state machine for this problem, a set of two or three functions would be a much better implementation.

  • (unregistered)

    For those that are defending the use of goto here as the obvious way to implement this state machine, I suggest comparing with some open source code that does a similar job.  See the "unquote" function here:

    http://svn.twistedmatrix.com/cvs/trunk/twisted/protocols/_c_urlarg.c?view=auto&rev=11454&root=Twisted
  • (unregistered)

    <FONT style="BACKGROUND-COLOR: #efefef">Some developers cannot be given any powerful language. It's just like giving a gun to a chimpanzee. You'd be lucky if the only thing they shoot is their own feet.</FONT>

  • (cs) in reply to

    People who started with C and then latter pick up a little C++ are considered dangerous.

    Actually, it seems this code was written by some die hard Assembler coder. Total lack of loops and heavy goto'ing can be remnants of assembler to C porting.

  • (unregistered)

    <FONT face="Courier New">#include <stdio.h>
    #include <stdlib.h></FONT>

    <FONT face="Courier New">#define THE_BASE 16</FONT>

    <FONT face="Courier New">int main(int argc, char **argv) {
            char *hex;
            int value;
            char ascii[16];
            if (argc < 2) return 1;
            hex = argv[1];
            /*** MAGIC LINE ***/</FONT><FONT face="Courier New">
            value = strtol(hex, NULL, THE_BASE);
            sprintf(ascii, "%d", value);
            printf("hex: %s, int: %d, ascii: %s\n", hex, value, ascii);
            return 0;
    }

    </FONT>
  • (unregistered) in reply to Katja


    In C++ a boolean is a boolean. You can promote from a boolean {false, true} and it will be 0/non-0. The difference is important. Any logical operation is of type bool, not int/etc. !size works (through type promotion) but is not "correct" and is bad form.

  • (cs) in reply to

    Personally, I think the label
    H2A_ACCEPT
    has the wrong name.

    Every return value ends up there. Why?? Why not just return the return value from where you are in the function? And if you want to do it this way for some obscure (or less obscure, I might be missing something) call your label
    H2A_RETURN
    or something. If you don't accept the input, don't goto (H2A_)Accept :P

    Drak

  • (cs) in reply to Drak

    I see now that there is a line between
    H2A_ACCEPT
    and the final return statement:
    *to = 0;

    My meager knowledge of C++ tells me this makes the value of the variable located at pointer 'to' equal to 0.

    Why???????? (I must be missing something)

    Drak

  • (unregistered)

    This code doesn't even quite work correctly.  For every %NN, it requires three characters of space in the destination buffer, when it only uses one.

  • (unregistered)

    The use of goto really hurts my brain.

  • (unregistered)

    Hi,

    (newbie on this site)
    My teacher C++ said (a long time ago in a galaxy far away):
    If you use goto, break or continue on your exam, you'll fail...
    enough said.

    (reply on some posts: it's not that something is NOT WRONG for the compiler, that it's 'OK'.  The developer after you would like to understand also)

  • (unregistered)

    Great routine, as in: great way to write a scanning statemachine. Though it could have been a lot smarter written. Hex to Asc can build on the fact that 2 characters in the input form 1 ascii code, as ascii is 0-127, i.e. a byte, and a byte is at most 2 characters in hex. As the hex string always has to have 2 characters per byte, the routine could just loop through the hex string, grab 2 characters at a time, calculate the real value of the digit (0-15), calculate the ascii value by 16*first value + last value and then put that value in the char pointered buffer.

    Of course, you could also use sscanf with the 2 characters, making the loop very small.

  • (unregistered) in reply to

    people who fear 'goto' don't know how to program. You shouldn't fear keywords, you should fear dumb teachers creating fear for several keywords. You can recognize these teachers when they open their mouth and rubbish like "only one returnpoint per function!" or "goto is evil!" or "if a function is longer than a screen, create 2 functions!" comes out. If that's the case, pack your bag and leave, as you then know you won't learn anything special from that bozo.

    FB, MVP C#

  • (unregistered) in reply to

    you probably replied on my post saying that goto/break/continue are evil.
    Evil is a big word, but those keywords do go into the whole O-O idea making C++ a language which can do much good if programmed correctly, but can give spaghetti-code all too often. (btw O-O has more to it than what the compiler considers O-O)

    There's always a way to design code without goto/break/continue so if you HAVE to use them, you just designed wrong.  The developer after you will have a heck of a time trying to figure out what the hell you're doing.

    Although I do agree that a function may be longer than the size of your screen...[:)]

  • (cs) in reply to

    ...you won't learn anything special from that bozo.

    FB, MVP C#

    I'd second that. Most of teachers in academy don't have any idea what they are talking about. Because in most of the cases they didn't write a single line of code in their life. They just repeat all that nonsense that their teachers told them and it never ends.

  • (cs) in reply to

    :
    There's always a way to design code without goto/break/continue so if you HAVE to use them, you just designed wrong.

    If you'll try really hard you'll be able to learn how to brush your teeth through nose. However, it doesn't mean you should actually do that. That's why you're getting f..king higher education - to learn how to think and how to do smart decisions; not to follow some shallow mantras.

  • (unregistered)

    Oh my god. I don't even remember when I last saw that many gotos at a glance. This guy obviously uses labels to comment his code. And what's the point in having labels if you don't write any jumps as well? Feeling a little bit dizzy now.

    Too bad there's no COME FROM in C.

  • (unregistered) in reply to AK-47
    AK-47:

    Actually, it seems this code was written by some die hard Assembler coder. Total lack of loops and heavy goto'ing can be remnants of assembler to C porting.

    I have NEVER written such spaghetti in asm. It's good to understand some principles of structural coding even when writing asm.

  • (unregistered)

    OK, here is my attempt. I hope it doesn't end up as tomorrow's entry... This code is placed in the public domain.


    /* 
     * Converts a string with embedded escape sequences to a plain string.
     * eg. "C%23" becomes "C#", "%24505" becomes "$505".
     */
    h2a_err_t hex2asc( char * to, const char * from, size_t size )
    {
        size_t idx_src  = 0;
        size_t idx_dest = 0;
        h2a_err_t rtn   = H2AERR_OK;

        while ( from[idx_src] != '\0' )
        {
            if (idx_dest + 1 >= size)
            {
                rtn = H2AERR_BUFFER_TOO_SMALL;
                break;
            }

            if (from[idx_src] != '%')
            {
                to[idx_dest++] = from[idx_src++];
            }
            else
            {
                char hex_str[3];

                strncpy(hex_str, &from[idx_src + 1], 2);
                hex_str[2] = '\0';

                if (2 != strlen(hex_str)  ||
                    !isxdigit(hex_str[0]) ||
                    !isxdigit(hex_str[1]))
                {
                    rtn = H2AERR_INVALID_HEX;
                    break;
                }

                to[idx_dest++] = (char) strtol(hex_str, NULL, 16);

                idx_src += 3;
            }
        }

        if (size == 0)
            rtn = H2AERR_BUFFER_TOO_SMALL;
        else
            to[idx_dest] = '\0';

        return rtn;
    }


  • (unregistered) in reply to AK-47

    If you'll try really hard you'll be able to learn how to brush your teeth through nose

    That comparison is just nonsense.

    to learn how to think and how to do smart decisions:

    idd and mine is, not to use break/goto/continue stuff [:)]
    The fact that you do is, off course your business.  I just hope I don't have to work with your code, but then again you probably think that of me too. (aah what a wonderful world)

  • (cs) in reply to Drak
    Drak:

    My meager knowledge of C++ tells me this makes the value of the variable located at pointer 'to' equal to 0.

    Why???????? (I must be missing something)

    "to" is a (moving) pointer to a character -- Specifically, it points to the place the next character to be written goes.  When we're done, it points to the end of the string.  By storing a zero there, we terminate the string.   I probably would have written that as:

    static const char END_OF_STRING = '\0';      // @ top of file
    // :
    *to = END_OF_STRING;

    But that's just me....

  • (cs) in reply to

    :
    This code doesn't even quite work correctly.  For every %NN, it requires three characters of space in the destination buffer, when it only uses one.

    Hmmmmm.... I hadn't noticed that.  Good catch.

    Now, that means, that if we keep that faux requirement, all we need do is check size against strlen(from) once at the top of the function, and then we can remove all other checking of size!

  • (cs) in reply to

    :
    My teacher C++ said (a long time ago in a galaxy far away):
    If you use goto, break or continue on your exam, you'll fail...
    enough said.

    OK, "goto" I understand.  "continue" maybe.... but "break"???

    It's very difficult to write a program that doesn't use break -- switch/case would pretty much be impossible, and exiting a while or for early without break would require rather contorted code.

  • (cs) in reply to AK-47

    AK-47:
    I'd second that. Most of teachers in academy don't have any idea what they are talking about. Because in most of the cases they didn't write a single line of code in their life.

    Actually, most of the computer teachers I know, do consulting jobs over the summer.

  • (unregistered) 
    <font color="#0000ff">if</font> ( ! --size ) {

    This is a VERY dangerous quagmire of  precendence.. I would re-write this. Even though C allows for shorthand, I'd only recommend the usage of x++; and x--; and NEVER in an comparison statement..

  • (cs) in reply to

    :
    OK, here is my attempt. I hope it doesn't end up as tomorrow's entry...
    Actually, it's quite good.  But being an anal retentive nit-picker, I have a few comments.  Mainly in the hex- conversion else clause, you're doing a lot of work to accomplish a little (I come from a asm/C background, so I'm always counting cycles)  Specifically, using strncpy to copy two bytes is definite overkill.  This could (almost) be written as:

                char hex_str[3];
                hex_str[0] = from[idx_src + 1];
                hex_str[1] = from[idx_src + 2];
                hex_str[2] = '\0';

    Now I say "almost" because if the string ended with a '%', then from[idx_src + 2] beyond the end of the string, and could be outside the memory partition for this process, causing a protection fault.  Now, this would mean that bad input would cause an error, which wouldn't be that bad, but it would be frowned upon.  However, a NUL in the wrong place would also be rejected by isxdigit, so if we could move the check first, and use that:

    if (isxdigit(from[idx_src + 1] && isxdigit(from[idx_src + 2])
    {
        char hex_str[3];
        hex_str[0] = from[idx_src + 1];
        hex_str[1] = from[idx_src + 2];
        hex_str[2] = '\0';
        // at this point, we've done the isxdigit check, the strlen check
        // and the strncpy, so all that's left is:
        to[idx_dest++] = (char) strtol(hex_str, NULL, 16);
        idx_src += 3;
    }
    else
    {
         rtn = H2AERR_INVALID_HEX;
         break;
    }

    This code is placed in the public domain.

    pet peeve.... Never place something in the "public domain".  You are giving up more rights than you think.  Better to just say "You may use it freely".

  • (cs)

    And I still disagree with the generic opinion here about the ! operator. By default, C and C++ doesn't even know any boolean types. It only recognizes if a value is zero or not. So things like !Size or !--Size are still proper usage of this operator.
    This is not just my opinion. This is just the C++ Standard! I can't help it if it seems a crappy standard, but it just is. C and C++ just doesn't know any boolean types to begin with.

  • (unregistered) in reply to

    #include <stdio.h>    /* puts */
    #include <stdlib.h>   /* strtol */
    #include <string.h>   /* memcpy */

    /*
     * Converts a string with embedded escape sequences to a plain string.
     * eg. "C%23" becomes "C#", "%24505" becomes "$505".
     */

    /*
     * Function does bounds checking, but expects a NULL terminated string.
     *
     * Returns 0 on OK. *to is invalid on an error.
     *
     * Compile with    cc -Wall hex2asc.c -o hex2asc
     *
     */

    int hex2asc(char *to, char *from, int size)
    {
    char *p=to;      /* Save original address of 'to' */
    char s[3]="..";  /* Use this store values. NULL term. the 3rd byte */

    while(*from)
     {
      if(*from=='%')
        {
        if(from[1] && from[2])        /* Check bounds         */
          {
          memcpy(&s, from+1, 2);      /* Copy hex chars       */
          *to=strtol(s, NULL, 16);    /* Convert              */
          to++;
          from=from+3;
          continue;                   /* Next char            */
          }
          else
          return 2;                   /* Error, % incomplete  */
        }

      *to=*from;                      /* Copy ordinary char   */
      to++;
      from++;

      if( (to-p) > size-1)            /* Check bounds         */
        return 1;                     /* Error, out of bounds */

      } /* End of while() */

    *to='\0';                         /* NULL terminate 'to'  */

    return 0;                         /* Return success       */
    }


    int main(void)
    {
    char *a="eg. \"C%23\" becomes \"C#\", \"%24505\" becomes \"$505\".";
    char b[64];

    if(hex2asc(b, a, 64))
      puts("Error");
      else
      puts(b);

    return 0;
    }



    Here's my humble entry.. took about 5 minutes to put together.. Seems to work ok. 7 years of ANSI C coding experience here ;)

      -- Ren

  • (cs)

    Something that hasn't been mentioned: <font face="Courier New">isxdigit</font> requires unsigned character codes (or -1) but <font face="Courier New">char</font> may be signed. If someone managed to include a byte with a value between 0x80 and 0xFE into the URL (I assume that's what "from" points to) then the argument to <font face="Courier New">isxdigit</font> will be invalid, with undefined results (but probably it will look at whatever random junk happens to precede its lookup table). This is unfortunately a fairly common error.

  • (unregistered) in reply to JamesCurran

    switch was the only place where we were allowed to use break ...

    (I should've mentioned that sorry)
    What we weren't allowed was something like this:

    for(int i = 0; i < somecount; i++){
    //blablabla code here
    if(someboolistrue){
    break;
    }
    }

    same for while and stuff

Leave a comment on “Yet Another &quot;Hex To Ascii&quot; ”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article