Unitialised value(s) using valgrind

Question

I'm running valgrind and I'm getting the following error.. before I made a backup I fixed it but now I don't remember how. Error was generated by malloc but I can't find the error in the code

>Insert password for admin: ==5720== Conditional jump or move depends on uninitialised value(s)
==5720==    at 0x40299EB: strcmp (mc_replace_strmem.c:538)
==5720==    by 0x80496C6: adm_log_request (commands_man.c:169)
==5720==    by 0x80521CA: main (mmboxman.c:9)
==5720==  Uninitialised value was created by a heap allocation
==5720==    at 0x4028876: malloc (vg_replace_malloc.c:236)
==5720==    by 0x8049683: adm_log_request (commands_man.c:165)
==5720==    by 0x80521CA: main (mmboxman.c:9)
==5720==

This is the function The line commands_man: 165 is that after if(size > 0)

int adm_log_request(void){

FILE *password;
char *pwdin, *frompwd = NULL;
int primo = 0/*indica se è un primo avvio*/, tentativi = 2, p, size;

if(!(password = fopen(F_PWD_ADM, "rb"))){
    primo = 1;
    printf("First server boot\n>Insert password for admin: ");
}
else{
    primo = 0;
    printf(">Insert password for admin: ");
}
p = get_hid_pass(&pwdin);
if(p < 0)
    return -1;
switch(primo){
    case 0:
        if(!(password = fopen(F_PWD_ADM, "r")))
            return -1;
        fread(&size, sizeof(int), 1, password);
        if(size > 0){
            frompwd = (char*)malloc(size + 1);
            fread(frompwd,sizeof(frompwd),1,password);
        }else return 0;
        while(tentativi > 0){
            if(strcmp(pwdin, frompwd) != 0){
                printf("\nIncorrect password\n%d attempts left\n>Insert password for admin: ", tentativi);
                tentativi--;
            }
            else return 1;
            p = get_hid_pass(&pwdin);
            if(p < 0)
                return -1;
        }
        fclose(password);
        break;
    case 1:     //primo avvio del server
        if(!(password = fopen(F_PWD_ADM, "w")))
            return -1;
        size = strlen(pwdin) + 1;
        fwrite(&size, sizeof(int), 1, password);
        fwrite(pwdin, sizeof(pwdin), 1, password);
        fclose(password);
        break;
}
if(tentativi == 0)
    return -1;

return 1;
}

Could someone help me to fix them?? Thank you

score 1 · Accepted Answer · answered Feb 09 '12 at 15:38

1

Part of the problem looks like an issue with sizeof:

        fread(frompwd,sizeof(frompwd),1,password);

In the above line, sizeof will have a value of 4 (assuming 32-bit architecture). It may be that you need to pass in size for the length. And then it still needs to be null terminated after that.

frompwd[size] = '\0';

The fwrite call has a similar problem and will only write 4 bytes of the password.

answered Feb 09 '12 at 15:38

Mark Wilkins

40,729
5
57
110

I find the problem..it was in size = strlen(pwdin) + 1; +1 was wrong!! Thanks!! =) – roccocullo Feb 09 '12 at 15:46
@roccocullo: It depends if you are wanting to write the null terminator byte or not. And note that the `fwrite` call still needs to be given the correct length (size) instead of the sizeof results. – Mark Wilkins Feb 09 '12 at 15:49

score 0 · Answer 2 · answered Feb 09 '12 at 15:40

Maybe fread doesn't actually set size to anything?

It's not guaranteed that it will. See its return value (from the man page):

RETURN VALUES

The functions fread() and fwrite() advance the file position indicator for the stream by the number of bytes read or written. They return the number of objects read or written. If an error occurs, or the end-of- file is reached, the return value is a short object count (or zero).

The function fread() does not distinguish between end-of-file and error; callers must use feof(3) and ferror(3) to determine which occurred. The function fwrite() returns a value less than nitems only if a write error has occurred.

If the return value is for example 0, then size is still uninitialized in line 165. A good practice would be to check what fread returns and confirm that the value was actually read successfully.

score 0 · Answer 3 · answered Feb 09 '12 at 15:45

First you read 4 bytes (or 8 on 64-bit) from a file:

fread(frompwd,sizeof(frompwd),1,password);

You probably didn't mean to use sizeof(frompwd) here.

Then you, compare using strcmp:

strcmp(pwdin, frompwd)

strcmp carries on comparing until one of the two strings contains a nul '\0' character. In this case you never nul-terminated your password string, hence the message.

You should a) use the right read size, and b) use strncmp to make sure you don't get buffer-overrun errors like this one.

Don't forget to allocate space for the nul-terminator. You need string size plus 1. — ams, Feb 10 '12 at 09:25

Unitialised value(s) using valgrind

3 Answers3