Using strncpy. Valgrind throws invalid read

Question

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.

I think identifiers must be long enough so the programmer can read the code without wondering what i was trying to do :P. — Matías Hernán García, Oct 30 '12 at 03:39
What length of identifiers that is ok, is a subjective coding-style topic. However, you should consider to always write your source code in English, as you will then get far better help when posting to sites like this, or some other support channel. — Lundin, Oct 30 '12 at 07:36

score 3 · Answer 1 · edited Jun 20 '20 at 09:12

3

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';

edited Jun 20 '20 at 09:12

Community

1
1

answered Oct 30 '12 at 03:06

Sergey Kalinichenko

714,442
84
1,110
1,523

1

`strncpy()` is very unlikely to be the right solution; see [this article](http://the-flat-trantor-society.blogspot.com/2012/03/no-strncpy-is-not-safer-strcpy.html) from my own blog. – Keith Thompson Oct 30 '12 at 06:23
@KeithThompson Absolutely - that is very true: a `memcpy` call would do just fine there, and be less misleading. – Sergey Kalinichenko Oct 30 '12 at 12:20

score 0 · Answer 2 · answered Oct 30 '12 at 03:07

0

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

answered Oct 30 '12 at 03:07

mcabral

3,524
1
25
42

score 0 · Answer 3 · edited May 23 '17 at 10:34

 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.

The -2 is because "fn" is two shorter than "fn()". – Grandpa Oct 30 '12 at 14:26 — Grandpa, Oct 30 '12 at 14:26

Using strncpy. Valgrind throws invalid read

3 Answers3