Returning char array fails without printf()

Question

I have a program which takes a user input, sends that user input as an argument to a function which makes a calculation, then returns the char array to the main() function to be output there.

The return (char *)&buf; works fine when a printf() statement is run. However, when there is no printf(), the return does not seem to work, as the main() function cannot output the returned value.

Here's the code:

#include <stdio.h>
#include <string.h>
#include <openssl/sha.h>

using namespace std;

char* hash_function(char* input_string)
{
    int i = 0;
    unsigned char temp[SHA_DIGEST_LENGTH]; //where we will store the SHA digest. length = 20
    char buf[SHA_DIGEST_LENGTH*2];

    memset(temp, 0x0, SHA_DIGEST_LENGTH);  //array of size 20 to store SHA1 digest
    memset(buf, 0x0, SHA_DIGEST_LENGTH*2); //array of size 2*20 to store hex result?

    SHA1((unsigned char *)input_string, strlen(input_string), temp);

    for(i=0; i<SHA_DIGEST_LENGTH; i++){
        sprintf((char*)&(buf[i*2]), "%02x", temp[i]);
        //element in temp[i] formatted with %02x and stored in buf[i*2]
    }

    //printf("In FUNCTION: %s\n", buf); //*************************************
    return (char *)&buf;
}

int main(int argc, char * argv[])
{
    if(argc != 2)
    {
        printf("Usage: %s <string>\n", argv[0]);
        return -1;
    }

    char *hash = hash_function(argv[1]);

    printf("Plaintext:\t%s\nSHA-1:\t\t%s\n\n", argv[1], hash);

    //FILE *file = fopen("temp_file.txt", "a+"); //open file to write to
    //fprintf(file, "Plaintext: %s\nSHA-1: %s\n\n", argv[1], buf);

    return 0;
}

The line which I've marked with asterisks is the print() line i'm referring to.

In order to compile, use g++ [file_name] -lcrypto -o [output] You may have to download the openssl/sha.h package.

Please do not expect commenters to "download the openssl/sha.h package." — Weather Vane, Apr 26 '17 at 23:02
You can add `static` specifier to buf, the value will stay until the next call. — theoden8, Apr 26 '17 at 23:02
@theoden Not a good suggestion in general. Leads to all sorts of problems such as invalid data for earlier calls when the function is called multiple times. It's not necessary and can be solved in much better ways (e.g. return dynamic memory and caller passes in buffer). — kaylum, Apr 26 '17 at 23:05
@kaylum, yes, `static` is like singleton, and does not return anything that needs to be used twice. No need for remembering to free it though or to pass a pointer as an argument. — theoden8, Apr 26 '17 at 23:09
the reason printf makes it work is because you entered the world of Undefined Behavior. UB means that anything can happen, including appearing to work sometimes. — pm100, Apr 26 '17 at 23:50

score 0 · Answer 1 · answered Apr 26 '17 at 23:05

0

you are returning a pointer to a buffer allocated on the stack. Once hash_buffer returns, the memory allocated to buf goes away. You need to allocate a buf on the heap with malloc. So, change your function:

char* hash_function(char* input_string)
{
    int i = 0;
    unsigned char temp[SHA_DIGEST_LENGTH]; //where we will store the SHA digest. length = 20
    char *buf = NULL;
    buf = malloc(SHA_DIGEST_LENGTH*2);
    if (buf == NULL) {
        return NULL;
    }

    memset(temp, 0x0, SHA_DIGEST_LENGTH);  //array of size 20 to store SHA1 digest
    memset(buf, 0x0, SHA_DIGEST_LENGTH*2); //array of size 2*20 to store hex result?

    SHA1((unsigned char *)input_string, strlen(input_string), temp);

    for(i=0; i<SHA_DIGEST_LENGTH; i++){
        sprintf((char*)&(buf[i*2]), "%02x", temp[i]);
        //element in temp[i] formatted with %02x and stored in buf[i*2]
    }
    return buf;
}

answered Apr 26 '17 at 23:05

bruceg

2,433
1
22
29

I read another thread which suggested this, but the poster was told that it's bad practice as its unclear who should be allowed to free that memory, the one who reserves or a different one? I'll go with it, I was just hoping to find a more "proper" way to go about fixing it. – Tawm Apr 26 '17 at 23:28
1

It is acceptable practice to allocate memory in the function, and expect the calling routine to free it. – bruceg Apr 26 '17 at 23:33
For improved robustness you can replace the memset calls by initializing `temp`, and using `calloc` for `buf` – M.M Apr 27 '17 at 00:17
1

The `(char *)` cast is redundant, and your loop writes past the end of the buffer by 1 byte – M.M Apr 27 '17 at 00:17
What @M.M says is true. I just updated the OP's code to use `malloc`. – bruceg Apr 27 '17 at 00:20

Returning char array fails without printf()

1 Answers1