CODEWITHSUNDEEP
cmallocvalgrindstrncpy
Matías Hernán García
Matías Hernán García

Reputation: 241

Using strncpy. Valgrind throws invalid read

I made this function:

void procesar_llamadaAFuncion(t_proceso *unProceso, char *sentencia){
    char *nombreFuncion = sentencia;
    char *nombreFuncionSinParentesis = NULL;

    string_trim(&nombreFuncion);
    nombreFuncionSinParentesis = malloc(sizeof(char)*(strlen(nombreFuncion)-2));
    strncpy(nombreFuncionSinParentesis, nombreFuncion, strlen(nombreFuncion)-2);

    puts(nombreFuncionSinParentesis);

    push_stack(unProceso->pcb->seg_stack, nombreFuncionSinParentesis, unProceso->pcb->program_counter);

    unProceso->pcb->program_counter = get_pos_funcion(unProceso->pcb->funciones, nombreFuncionSinParentesis);

    free(nombreFuncion);
    free(nombreFuncionSinParentesis);

It doesn't matter what t_proceso is, the problem is that this function receives an array of chars.

The array of chars that the function will receive its always "something()", what i am trying to do is to remove the two last characters "()" and then call the function push_stack().

The problem is that when I run Valgrind, i get this:

==17129== Invalid read of size 1
==17129==    at 0x4C2BFD4: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17129==    by 0x50BFCEB: puts (ioputs.c:37)
==17129==    by 0x403D30: procesar_llamadaAFuncion (proceso.c:455)
==17129==    by 0x40313D: procesar_siguiente_instruccion (proceso.c:132)
==17129==    by 0x404B1A: probarProcesos (test.c:83)
==17129==    by 0x404C7F: main (test.c:111)
==17129==  Address 0x5436da8 is 0 bytes after a block of size 8 alloc'd
==17129==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17129==    by 0x403CDF: procesar_llamadaAFuncion (proceso.c:452)
==17129==    by 0x40313D: procesar_siguiente_instruccion (proceso.c:132)
==17129==    by 0x404B1A: probarProcesos (test.c:83)
==17129==    by 0x404C7F: main (test.c:111)

I don't know what I am doing wrong, so any help will be appreciated.

Upvotes: 4

Views: 1716

Answers (3)

Lundin
Lundin

Reputation: 214385

 nombreFuncionSinParentesis = malloc(sizeof(char)*(strlen(nombreFuncion)-2));

The above is not correct, you need to allocate room for the null termination as well. I don't understand what the -2 does, but add +1 byte to whatever you are attempting, that is

malloc(sizeof(char)*(strlen(nombreFuncion)-2 + 1));

As suggested in a comment, strncpy should not be used, it is an obscure function written for specific needs of ancient versions of unix. Read this post or indeed the nice article posted as a comment to another answer.

free(nombreFuncion);

The above is very suspicious, you are doing free(sentencia) which was allocated outside the function. If that was the intention, you should consider a better program design.

Upvotes: 0

mcabral
mcabral

Reputation: 3558

You have to null terminate your strings. That counts for the param "sentencia" and for the result of removing the parenthesis

Upvotes: 0

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726809

This is because strncpy does not null-terminate the destination string:

Copies the first num characters of source to destination. If the end of the source C string (which is signaled by a null-character) is found before num characters have been copied, destination is padded with zeros until a total of num characters have been written to it.

No null-character is implicitly appended at the end of destination if source is longer than num (thus, in this case, destination may not be a null terminated C string).

This should fix the problem:

size_t nobmreLen = strlen(nombreFuncion)-2;
// Don't forget to add +1 for the null terminator
nombreFuncionSinParentesis = malloc(sizeof(char)*(nobmreLen+1));
strncpy(nombreFuncionSinParentesis, nombreFuncion, nobmreLen);
nombreFuncionSinParentesis[nobmreLen] = '\0';

Upvotes: 3

Related Questions