Converting ASCII to Hex and vice versa - strange issue

Question

Working on writing two functions - one to convert ascii to hexadecimal and then vice-versa. Encountered something very very odd... With the printf(); statement commented out in the Asc2Hex function, it doesn't work. If I uncomment it, it works... Any idea? And if anyone knows of a better way to do this conversion, please do let me know.

#include <stdio.h>
#include <string.h>

char *Asc2Hex(char *);

int main()
{

    char *test = Asc2Hex("ABCDEFG");
    printf("Test: %s\n",test);

}


char *Asc2Hex(char *data){
    int i, len = strlen(data);
    char buffer[len+1];
    char *pbuffer = buffer;
    //printf("String: %s\n",data);
    for(i = 0; i < (len * sizeof(char)); i++){
        sprintf(pbuffer+i*2, "%x",*(data+i));
    }
    return pbuffer;
}

Some programmer dude · Answer 1 · 2014-09-02T18:37:40.207

You have undefined behavior because you return a pointer to a local variable. When the function Asc2Hex returns, the variable buffer goes out of scope, and the pointer to it that you return is not valid.

The safest solution is to have two extra arguments to the function, the buffer and its size (so you don't write beyond the bounds). Another solution, safe but not so good, is to make buffer a static variable, then its lifetime is the whole of the program and you can safely return a pointer to it.

mpromonet · Answer 2 · 2014-09-02T19:38:39.430

1

Fixing the allocation problem, your code could become something like this :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *Asc2Hex(char *data) {
    int i, len = strlen(data);
    char *buffer = malloc(2*len+1);
    if (buffer) {
        for(i = 0; i < len; i++){
            sprintf(&buffer[i*2],"%.2x",data[i]&0xff);
        }
    }
    return buffer;
}

int main() {    
    char *test = Asc2Hex("\xff");
    printf("Test: %s\n",test);
    if (test) free(test);
}

edited Sep 02 '14 at 19:38

answered Sep 02 '14 at 18:44

mpromonet

11,326
43
62
91

doh! I don't know how I missed that... Thank you everyone for your input. – Jonny Whatshisface Sep 02 '14 at 23:09

score 0 · Answer 3 · answered Sep 02 '14 at 18:28

0

Returning a pointer to a local variable is bad news, and causes undefined behaviour.

Editorial note: sizeof(char) is 1.

answered Sep 02 '14 at 18:28

Carl Norum

219,201
40
422
469

score 0 · Answer 4 · answered Sep 02 '14 at 18:44

As already identified, your code tries to use a pointer to a local variable after the function defining it returns, which is undefined behaviour and any result is valid according to the C standard, including 'all files on your hard disk are erased'. Avoid undefined behaviour!

The 'better' ways to do it include passing the output buffer to the conversion function, ensuring that you print 2 hex digits per input character, and that if plain char is a signed type, you still get the expected result:

#include <stdio.h>
#include <string.h>

char *Asc2Hex(const char *data, char *buffer);

int main(void)
{
    char  buffer[2*sizeof("ABCDEFG")];
    char *test = Asc2Hex("ABCDEFG", buffer);
    printf("Test: %s\n", test);
    return 0;
}

char *Asc2Hex(const char *data, char *buffer)
{
    int len = strlen(data);
    for (int i = 0; i < len; i++)
        sprintf(buffer+i*2, "%.2x", data[i] & 0xFF);
    return buffer;
}

This isn't ideal; it is not proof against overlong strings and undersized buffers. To fix that, you'd pass the buffer length into the function and ensure that the buffer was not overflowed. Returning the pointer passed in is not really needed with the revised code, but you need some way to return success/failure if you pass in a buffer length.

(Technically, it allocates one byte of memory that isn't needed for the buffer in main() — tell me of a computer where that actually matters!)

Converting ASCII to Hex and vice versa - strange issue

4 Answers4

Linked