2

LAST EDIT in the end of OP

I tested with Valgrind a function used in a project and it says "Source and destination overlap in memcpy" and gives me also "Invalid read" and "Invalid write" errors. I fixed the code in order to not overlap those two buffers but with no results. Here's the code:

static
int download_build_buffer(char **seq_numbered_buffer, int seq_n, int dim_payload, char *buffer) {

if(!seq_numbered_buffer)
    return 1;

/* allocates seq_numbered_buffer */
if(*seq_numbered_buffer != NULL) {
    free(*seq_numbered_buffer);
    *seq_numbered_buffer = NULL;
}
if(!(*seq_numbered_buffer = malloc((SIZE_SEQ_N + SIZE_DIM_S + dim_payload + 1) * sizeof(char))))
    return 1;

#if DEBUG_DOWNLOAD
fprintf(stderr, "download_build_buffer %d: seq->%d, dim->%d\n",
                getpid(), seq_n, dim_payload);
#endif

/* prints sequence number in its string */
seq_n = htonl(seq_n);
if(!memcpy(*seq_numbered_buffer, &seq_n, SIZE_SEQ_N))
    return 1;
dim_payload = htonl(dim_payload);
if(!memcpy(&(*seq_numbered_buffer)[SIZE_SEQ_N], &dim_payload, SIZE_DIM_S))
    return 1;

/* creates payload -> buffer = seq_number + buffer */
if(!memcpy(&(*seq_numbered_buffer)[SIZE_SEQ_N+SIZE_DIM_S], buffer, dim_payload))
    return 1;
(*seq_numbered_buffer)[SIZE_SEQ_N+SIZE_DIM_S+dim_payload] = 0;

return 0;
}

This function is being called with the following:

/* allocates buffer */
if(buffer != NULL) {
    free(buffer);
    buffer = NULL;
}
assert((buffer = malloc((dim_payload+1)*sizeof(char)))
               != NULL);

/* stores bytes in buffer */
if((n = read(fd, buffer, dim_payload)) <= 0)
    break;
buffer[n] = 0;
buf_len = n;

/* increments seq_number */
seq_n = next_request;

/* creates payload -> buffer = seq_number + buffer */
if(download_build_buffer(&seq_numbered_buffer, seq_n, dim_payload, buffer))
    return 1;

where seq_numbered buffer and buffer are defined and initialized as:

char *seq_numbered_buffer = NULL,
     *buffer = NULL;

also, I've tried to replace memcpy with memmove but with no results :( My Valgrind output is

==3921== Source and destination overlap in memcpy(0x41bb2b0, 0x41bb070, 131072)
==3921==    at 0x4027BD6: memcpy (mc_replace_strmem.c:635)
==3921==    by 0x8049A8D: download_build_buffer (uftp_server.c:791)
==3921==    by 0x8049F4C: server_download_file (uftp_server.c:989)
==3921==    by 0x804A4E1: data_connection_proc (uftp_server.c:1209)    
==3921==    by 0x804AEE8: main (uftp_server.c:1512)
==3921== 
==3921== Invalid read of size 4
==3921==    at 0x4027D42: memcpy (mc_replace_strmem.c:635)
==3921==    by 0x8049A8D: download_build_buffer (uftp_server.c:791)
==3921==    by 0x8049F4C: server_download_file (uftp_server.c:989)
==3921==    by 0x804A4E1: data_connection_proc (uftp_server.c:1209)
==3921==    by 0x804AEE8: main (uftp_server.c:1512)
==3921==  Address 0x41db06c is not stack'd, malloc'd or (recently) free'd
==3921== 
==3921== Invalid write of size 4
==3921==    at 0x4027D44: memcpy (mc_replace_strmem.c:635)
==3921==    by 0x8049A8D: download_build_buffer (uftp_server.c:791)
==3921==    by 0x8049F4C: server_download_file (uftp_server.c:989)
==3921==    by 0x804A4E1: data_connection_proc (uftp_server.c:1209)
==3921==    by 0x804AEE8: main (uftp_server.c:1512)
==3921==  Address 0x41db2ac is not stack'd, malloc'd or (recently) free'd
==3921== 
==3921== Invalid write of size 1
==3921==    at 0x8049A9C: download_build_buffer (uftp_server.c:792)
==3921==    by 0x8049F4C: server_download_file (uftp_server.c:989)
==3921==    by 0x804A4E1: data_connection_proc (uftp_server.c:1209)
==3921==    by 0x804AEE8: main (uftp_server.c:1512)
==3921==  Address 0x41db2b0 is not stack'd, malloc'd or (recently) free'd
==3921== 
server_download_file 3921: download finished
--3921-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--3921-- si_code=1;  Faulting address: 0x0;  sp: 0x62af9e0c

valgrind: the 'impossible' happened:
Killed by fatal signal
==3921==    at 0x38034171: unlinkBlock (m_mallocfree.c:244)

sched status:
running_tid=1

Thread 1: status = VgTs_Runnable
==3921==    at 0x4026864: malloc (vg_replace_malloc.c:236)
==3921==    by 0x804911B: file_release_lock (uftp_server.c:423)
==3921==    by 0x8049DE9: server_download_file (uftp_server.c:930)
==3921==    by 0x804A4E1: data_connection_proc (uftp_server.c:1209)
==3921==    by 0x804AEE8: main (uftp_server.c:1512)

In case, ask me for other informations, thanks

EDIT Before last memcpy dim_payload is transformed with htonl() to network format and passes from 512 to 131072.. It needs to return 512 by using ntohl().

gc5
  • 9,468
  • 24
  • 90
  • 151
  • if(!memcpy(*seq_numbered_buffer, &seq_n, SIZE_SEQ_N)) memcpy doesn't return an error condition. It just returns the value of its first argument. Who knows what it will do if the value of the first argument is NULL (it might just crash). – davep Jun 11 '11 at 21:05
  • First argument cannot be NULL because I added a check right before memcpy that make the function return if it is NULL – gc5 Jun 12 '11 at 13:44
  • Not unexpected. That means the "if" and "returns" don't DO anything! (It also isn't clear whether you do the odd "if" and "return" code elsewhere). Why do you have that code if it doesn't do anything? – davep Jun 12 '11 at 17:37
  • I don't get it.. NULL is defined as (void *)0.. if I test !0 is 1: it will execute "return 1;".. – gc5 Jun 16 '11 at 22:15

1 Answers1

6

You should not have side effects in assert. When compiling without them, they expression is simply ignored. In this case, the following might cause your problem:

assert((buffer = malloc((dim_payload+1)*sizeof(char)))
           != NULL);

I would replace it with:

buffer = malloc((dim_payload+1)*sizeof(char));
assert(buffer != NULL);

Oh, and, sizeof(char) is always 1, so you can drop that.

Lindydancer
  • 25,428
  • 4
  • 49
  • 68
  • 2
    +1. I would add that `assert` is not the right thing to use for testing `malloc`'s success. `assert(condition)` should be only used where `condition` is *always* true. – Alok Singhal Jun 11 '11 at 21:31
  • weird thing.. I changed it but nothing happens :| by the way I get no problem running it while not debugging.. I add my Valgrind output in the OP above – gc5 Jun 12 '11 at 13:38