StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>You don't initialise <code>temp</code> and, on top of that, your call to <code>realloc</code> is wrong. It should be:</p> <pre><code>temp = realloc (temp, recvsize+1); </code></pre> <p>When you call <code>realloc</code> as you have, you throw away the new address and there's a good chance that the old address has now been freed. All bets are off when you then try to dereference it.</p> <p>The reason <code>realloc</code> returns a new address is because expansion of the buffer may necessitate it being moved if that current block is surrounded in the memory arena (in other words, it can't just expand into a free block following it). In that case, a new block will be created in the arena, the contents transferred from the old block and the old block freed. You have to get the return value from <code>realloc</code> in case that happens.</p> <p>Keep in mind that <code>realloc</code> doesn't <em>have</em> to return a new pointer, it may give you the same pointer if, for example, there was enough free space after the block to satisfy the new size or if you're reducing the size.</p> <p>It can also return NULL if it can't expand the block, you should watch out for that as well, especially since:</p> <pre><code>temp = realloc (temp, newsize); </code></pre> <p>will result in a memory leak when it returns NULL (it doesn't free the old block).</p> <p>A few other things:</p> <ul> <li>you rarely need to use <code>calloc</code>, especially in this case since you're copying over the memory anyway.</li> <li>similarly, you don't need to <code>memset</code> a memory chunk to 0 if you're immediately going to <code>memcpy</code> over it.</li> <li>provided you initialise <code>temp</code> to <code>NULL</code>, you can just use <code>realloc</code> without testing it. That's because <code>realloc(NULL,7)</code> is identical to <code>malloc(7)</code> - <code>realloc</code> is perfectly capable of starting with a null pointer.</li> <li>since you don't need <code>calloc</code>, this is for education only - <code>sizeof(char)</code> is <em>always</em> 1 by definition.</li> <li>you seem to be doing an awful lot of unnecessary copying of data.</li> </ul> <hr> <p>Why don't we start with something a bit simpler? Now, this is totally from my head so there may be some bugs but it's at least cut down from the memory-moving behemoth in the question :-) so should be easier to debug.</p> <p>It's basically broken down into:</p> <ul> <li>initialise empty message.</li> <li>enter infinite loop. <ul> <li>get a segment.</li> <li>if error occurred, free everything and return error.</li> <li>if no more segments, return current message.</li> <li>create space for new segment at end of message.</li> <li>if no space could be created, free everything and return empty message.</li> <li>append segment to message and adjust message size.</li> </ul></li> </ul> <p>and the code looks like this:</p> <pre><code>int Socket::Recv(char *&vpszRecvd) { int recvsize = 0; char TempBuf[1024]; int Result = 0; char *oldPtr; // Optional free current and initialise to empty. //if (vpszRecvd != NULL) free (vpszRecvd); vpszRecvd = NULL; // Loop forever (return inside loop on end or error). do { Result = recv( this->sSocket, TempBuf, sizeof(TempBuf) -1, 0 ); // Free memory, close socket on error. if (Result < 0) { free (vpszRecvd); closesocket(this->sSocket); this->sSocket = INVALID_SOCKET; return SOCKET_ERROR; } // Just return data and length on end. if (Result == 0) { return recvsize; } // Have new data, use realloc to expand, even for initial malloc. oldPtr = vpszRecvd; vpszRecvd = realloc (vpszRecvd, recvsize + Result); // Check for out-of-memory, free memory and return 0 bytes. if (vpszRecvd == NULL) { free (oldPtr); return 0; } // Append it now that it's big enough and adjust the size. memcpy (&(vpszRecvd[recvsize], TempBuf, Result); recvsize += Result; } while (1); } </code></pre>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload