Memory corruption on freeing a pointer to pointer

[Keith Thompson]

On Sat, 24 Aug 2013 12:05:02 -0700 (PDT), Sharwan Joram

strncpy(parameters[parametercount], token, strlen(token));

[...]

strcpy would be more efficient than strncpy for copying the string
unless you change the third argument to something useful.

And strncpy is *not* just a "safer" strcpy; it can leave the target
array without a terminating '\0' character (i.e., not a string).

 

[Malcolm McLean]

What are the options? Are you assuming a reader who thinks sizeof might
be a variable?

A bad option is

buff = malloc( N * sizeof *buff);

it's got two C quirks - an operator which looks like an identifier, and a
sort of pointer dereference which isn't really a dereference. It also uses the
asterisk in two contexts, which aren't visually distinguishable. It's asking
for trouble, difficulty in understanding, adding cost to the maintenance of
programs.

buff = malloc( N * sizeof(*buff));

is a bit less bad. At least we can see that sizeof applies to *buff, and that
* is unary. A novice or an experience programmer unused to C might assume
that sizeof() is a fucntion, but that's unlikely to do any real harm.

The best option is

buff = malloc(N * sizeof(int));

it's very easy to see what the intention is.

 

[Ian Collins]

A bad option is

buff = malloc( N * sizeof *buff);

it's got two C quirks - an operator which looks like an identifier, and a
sort of pointer dereference which isn't really a dereference. It also uses the
asterisk in two contexts, which aren't visually distinguishable. It's asking
for trouble, difficulty in understanding, adding cost to the maintenance of
programs.

If a so called programmer doesn't understand this idiomatic construct,
they shouldn't be working on C code.

buff = malloc( N * sizeof(*buff));

Most programmers would be happy with and probably use this form.

is a bit less bad. At least we can see that sizeof applies to *buff, and that
* is unary. A novice or an experience programmer unused to C might assume
that sizeof() is a fucntion, but that's unlikely to do any real harm.

The best option is

buff = malloc(N * sizeof(int));

it's very easy to see what the intention is.

Until the type of buff changes, which does happen.

 

[Malcolm McLean]

Malcolm McLean wrote:

If a so called programmer doesn't understand this idiomatic construct,
they shouldn't be working on C code.

A programmer can be a very experienced programmer, but not so familiar with C.
That's happening more and more with the explosion of high-level languages.
But even an experienced C programmer thinks in English (assuming him to be
an anglophone). "size of int" is prounceable in English, and has the correct
meaning. size of star peeteear doesn't have a meaning.
In programming the trivial is important. Little difficulties have a way of
accumulating, and combining to make major headaches.

Until the type of buff changes, which does happen.

Automatic variables shouldn't have a large scope, so you should be able to
check the entire function when making a modification. The real danger is
when buff is a member of a structure, which has many functions which operate
on it. But you can easily have other problems.

 

[Ben Bacarisse]

I've modified the code with the guidelines given by James and other
mentors in the list, mentioned below. But it's still failing.

Some things not yet mentioned...

-----------Code Snippet -------------




/*-----------------------------------------*/
if(token != NULL){
char *temp_token = (char *) malloc (strlen(saved_token) + 1);
memcpy(temp_token, saved_token, strlen(saved_token));
idx = parametercount = detect_delim_count(temp_token, delimiters);
if (temp_token)
free(temp_token);
parameters = malloc(parametercount * sizeof(char *));
if ( NULL == parameters){
fprintf(stdout, "CLI: Couldn't allocate sufficient memory . Exiting \n");
exit(1);
}
for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

This loop can run off the end of the allocated array. You need to stop
before parameterCount == idx.

parameters[parametercount] = calloc(30 , strlen(token) + 1);
if ( NULL == parameters[parametercount]){
fprintf(stdout,"CLI: Couldn't allocate sufficient memory. Exiting \n");
exit(1);
}
strncpy(parameters[parametercount], token, strlen(token));
}
shutdown = (currentcommand->command_handler)(client_fd, parameters, parametercount);
}
}else{
/* some more code here */

}
if (parameters != NULL){
for (parametercount = idx ; parametercount >= 0; --parametercount)
free(parameters[parametercount]);

Here you free elements that you never set. Even if you used every
available element, the very first free call accesses memory outside the
array.

free(parameters);
}

<snip>

 

[Ben Bacarisse]

A bad option is

buff = malloc( N * sizeof *buff);

No. My remark followed this (no snipped):

| At least put in parentheses so we can have visual fix on what sizeof
| applies to.

I meant what are the options for what sizeof may apply to? I can see
only three, and the wrong ones are so daft I would bother to address
them.

<snip>

 

[Keith Thompson]

A bad option is

buff = malloc( N * sizeof *buff);

it's got two C quirks - an operator which looks like an identifier, and a
sort of pointer dereference which isn't really a dereference. It also uses the
asterisk in two contexts, which aren't visually distinguishable. It's asking
for trouble, difficulty in understanding, adding cost to the maintenance of
programs.

You understand it perfectly well, don't you?

sizeof is unusual in that it's the only operator named by a keyword
rather than by a punctuation symbol. (C11 adds _Alignof, but that
doesn't take a subexpression operand.) There are other languages that
have multiple operators whose names are keywords. Even C has "and",

buff = malloc( N * sizeof(*buff));

is a bit less bad. At least we can see that sizeof applies to *buff, and that
* is unary. A novice or an experience programmer unused to C might assume
that sizeof() is a fucntion, but that's unlikely to do any real harm.

A programmer who thinks sizeof is a function needs to learn that sizeof
isn't a function. Once. You might try telling people that yourself,
rather than suggesting elaborate workarounds to avoid the burden of
learning it.

I have no great objection to extraneous parentheses on sizeof; they're
mostly harmless. But I dislike using them myself, because I prefer to
keep in mind that sizeof is an operator, not a function. (Same thing
for "return" and "case".)

The best option is

buff = malloc(N * sizeof(int));

it's very easy to see what the intention is.

No, because I declared "int32_t *buff", and if I write it that way I'll
never notice the error until I try it on a 64-bit system.

(And someone with the level of expertise you assume will probably think
that sizeof is a function, and wonder what it means to evaluate "int" as
an argument.)

 

[Ian Collins]

A programmer can be a very experienced programmer, but not so familiar with C.
That's happening more and more with the explosion of high-level languages.

While true, that doesn't detract from my comment. programmers coming
from a scripting language probably aren't familiar with low level memory
management at all, so they have quite a bit to learn. The include C's
idioms. Suggesting experienced programmers avoid C idioms that may be
unfamiliar to others is a step too far.

But even an experienced C programmer thinks in English (assuming him to be
an anglophone). "size of int" is prounceable in English, and has the correct
meaning. size of star peeteear doesn't have a meaning.
In programming the trivial is important. Little difficulties have a way of
accumulating, and combining to make major headaches.

Such as the types of variables changing?

Automatic variables shouldn't have a large scope, so you should be able to
check the entire function when making a modification. The real danger is
when buff is a member of a structure, which has many functions which operate
on it. But you can easily have other problems.

I'd wager more dynamic allocations *are* used to initialise members of
structures (which are more likely to changer their type) than are used
for automatic variables. The only real use for dynamic allocations to
automatic variables is for objects that are too large for the (mythical)
stack.

 

[Sharwan Joram]

Some things not yet mentioned...












This loop can run off the end of the allocated array. You need to stop

before parameterCount == idx.

idx contains the number of delimiters available in the string. For example if the string contains 2 delimiters then the sub-strings are 3. The idx decremented by -1 while returning because of the increment in it's value by 1 in last iteration of for loop. This loop works well as expected.

parameters[parametercount] = calloc(30 , strlen(token) + 1);

if ( NULL == parameters[parametercount]){

fprintf(stdout,"CLI: Couldn't allocate sufficient memory. Exiting \n");

exit(1);

}

strncpy(parameters[parametercount], token, strlen(token));

}

shutdown = (currentcommand->command_handler)(client_fd, parameters, parametercount);

}

}else{

/* some more code here */

}

if (parameters != NULL){

for (parametercount = idx ; parametercount >= 0; --parametercount)

free(parameters[parametercount]);

Here you free elements that you never set. Even if you used every

available element, the very first free call accesses memory outside the

array.

The elements are assigned above in the code. In for loop.

<snip>

Let me know if you find anything else suspicious which can resolve this problem.

 

[Sven Köhler]

Dear Sharwan,

The code below doesn't crash and is tested with valgrind.
Learn from it.


#include <string.h>
#include <stdlib.h>
#include <stdio.h>

int detect_delim_count(char *i_str, const char *delim) {
char *token;
int count = 0;

for (token = strtok(i_str, delim); token && *token; token =
strtok(NULL, delim))
count++;

return count;
}

int main() {

char **parameters;
char *token;
int idx, parametercount;

const char* saved_token = "d e f";
const char* delimiters = " ";

char *temp_token = (char *) malloc (strlen(saved_token) + 1);
if ( NULL == temp_token){
fprintf(stdout, "CLI: Couldn't allocate sufficient memory . Exiting \n");
exit(1);
}
strcpy(temp_token, saved_token);
parametercount = detect_delim_count(temp_token, delimiters);
strcpy(temp_token, saved_token);

parameters = malloc(parametercount * sizeof(char *));
if ( NULL == parameters){
fprintf(stdout, "CLI: Couldn't allocate sufficient memory . Exiting \n");
exit(1);
}
for( idx = 0 , token = strtok(temp_token, " "); token && *token ; ++idx
, token = strtok(NULL, " ")){
parameters[idx] = malloc(strlen(token) + 1);
if ( NULL == parameters[idx]){
fprintf(stdout,"CLI: Couldn't allocate sufficient memory. Exiting \n");
exit(1);
}
fprintf(stdout, "found token %s\n", token);
strcpy(parameters[idx], token);
}
free(temp_token);
//shutdown = (currentcommand->command_handler)(client_fd, parameters,
parametercount);

while (parametercount > 0)
free(parameters[--parametercount]);
free(parameters);
}

 

[James Kuyper]

On 08/24/2013 03:05 PM, Sharwan Joram wrote:
....

I've modified the code with the guidelines given by James and other mentors in the list, mentioned below. But it's still failing.

Not quite. As I and other have said, for best results you need to give
us a complete, compilable program. All you've given us is a:

-----------Code Snippet -------------

While the code you have given us contains defects severe enough to be
the cause of the symptoms you've observed, you had no way of being sure
that would be the case. It could very well have been that the actual
problem was in some other part of your program.

if(token != NULL){
char *temp_token = (char *) malloc (strlen(saved_token) + 1);
memcpy(temp_token, saved_token, strlen(saved_token));

Once again, you're using the value returned by malloc() before bothering
to check whether malloc() has succeeded. If malloc() failed, temp_token
would be null, and the call to memcpy() would render the behavior of
your program undefined.

idx = parametercount = detect_delim_count(temp_token, delimiters);
if (temp_token)

Now you've finally bothered checking whether the allocation succeeded.
That's ironic, because you wait until after it is used twice in ways
that would be dangerous if temp_token is null. The only code you protect
is the call to free(), and free()ing a null pointer is actually
perfectly safe.

detect_delim_count() returns the number of delimiters. The number of
parameters is inherently one more than the number of delimiters. I see
from a later message that you're well aware of that fact. Therefore, I
find it puzzling that you would store the value returned by
detect_delim_count(), in a variable named parametercount. You should
either have named the variable delimitercount, or you should have added
1 to the value returned by detect_delim_count() before storing it in
parametercount.

free(temp_token);
parameters = malloc(parametercount * sizeof(char *));

Here you allocate enough room in the parameters array to store one
pointer for each delimiter. However, what you need is enough room to
store one pointer for each parameter, which is one more than the value
currently stored in parametercount.

if ( NULL == parameters){
fprintf(stdout, "CLI: Couldn't allocate sufficient memory . Exiting \n");
exit(1);
}
for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){
parameters[parametercount] = calloc(30 , strlen(token) + 1);

You're now allocating enough space for 30 copies of the token. I think
you should change 30 to 1, unless there's some reason, not obvious in
the code you've shown, for setting aside that much space.

The last successful call to strtok() will result in the above code
attempting to store the value returned by malloc() into parameters[idx],
but you only allocated enough memory to store idx pointers:
parameters[0] through parameters[idx-1]. You haven't allocated enough
space to store anything in parameters[idx]. Attempting to do so renders
the behavior of your program undefined. Under certain circumstances,
your program might fail immediately because of this defect. However,
judging from the symptoms you've reported, what you did was corrupt some
memory being used by some other part of the program, almost certainly
memory being used by the malloc() family to manage the heap. That's why
the subsequent call to free() failed.

if ( NULL == parameters[parametercount]){
fprintf(stdout,"CLI: Couldn't allocate sufficient memory. Exiting \n");
exit(1);
}
strncpy(parameters[parametercount], token, strlen(token));

strlen() has to examine every character in the string, to look for the
terminating null character. strncpy() needs to retrieve the value of
every character in the string. If you'd simply called strcpy() instead,
then it would only have gone through the string once.

In some contexts, strncpy(dest, src, count) can be safer than
strcpy(dest,src). That's true only when "count" is less than strlen(src)
and also less than the length of the array pointed at by dest.
strncpy(dest, src, strlen(src)) stops at exactly the same place that
strcpy() would stop - you don't get any protection at all. If
strlen(src) is bigger than the size of the array pointed at by dest,
strncpy(dest,src,strlen(src)) will still overwrite then end of that
array, just like strcpy().

}
shutdown = (currentcommand->command_handler)(client_fd, parameters, parametercount);
}
}else{
/* some more code here */

}
if (parameters != NULL){
for (parametercount = idx ; parametercount >= 0; --parametercount)
free(parameters[parametercount]);

You set parametercount initially to the same value as idx, and used
parametercount to determine how many parameters to allocate space for.
Unless code that you haven't shown has changed the value of idx since
that time, space was therefore only allocated for idx different
pointers, starting with parameters[0] and ending with parameters[idx-1].

However, the above loop starts by trying to retrieve the value of
parameters[idx] and passing it to free(). This would still be a separate
problem even if you corrected parametercount earlier to contain the
count of the parameters, rather than the count of delimiters. In that
case, parameters[idx] would refer to memory that had never been
allocated for use by your program, and which had never been written. to.
As a result, it might contain a trap representation, in which case even
retrieving the value of parameters[idx] would render the behavior of
your program undefined. Even it if were not a trap representation, it
almost certainly would not contain a value returned by a call to
malloc(). Therefore, passing that value to free() would also render the
behavior of your code undefined.

You should correct the for() loop to either of the following forms:

for(parametercount=idx-1; parametercount >=0; --parametercount)

or

for(parametercount=0; parametercount<idx; parametercount++)

The second form is slightly shorter and more idiomatic. I'd also
recommend exchanging the variable names idx and parametercount, since
you're using parameter count more like an index into the parameters
array, and less like a fixed count of parameters.

 

[Malcolm McLean]

Such as the types of variables changing?

The fact that the sizeof *ptr method is robust to the type of ptr changing is
a real advantage. That has to be balanced against the disadvantage of writing
code which is difficult for a human to read.

I'd wager more dynamic allocations *are* used to initialise members of
structures (which are more likely to changer their type) than are used
for automatic variables. The only real use for dynamic allocations to
automatic variables is for objects that are too large for the (mythical)
stack.

Stacks are getting very big now. I wouldn't have considered declaring a
buffer of 1000 ints on the stack when I started with C, now I routinely do
so.

 

[Ben Bacarisse]

You need to copy one more byte to ensure the string is null-terminated,
though, in fact, a simple strcpy(temp_token, saved_token); call is
better here.

This test is a bit late. I'd put it here:

char *temp_token = malloc(strlen(saved_token) + 1);
if (temp_token) {
strcpy(temp_token, saved_token);
idx = parametercount = detect_delim_count(temp_token, delimiters);
free(temp_token);
}
else { /* complain and exit */ }

You count delimiters using a variable "delimiters", but here you use
" ". This may be fine for you test case but it's not right in general
and could do all kinds of damage.

idx contains the number of delimiters available in the string. For
example if the string contains 2 delimiters then the sub-strings are
3. The idx decremented by -1 while returning because of the increment
in it's value by 1 in last iteration of for loop. This loop works well
as expected.

The one in detect_delim_count works but this one doesn't.

I'd missed the counting, but it still does not work. When there are no
delimiters, idx is set to 0 and you call malloc(0). That's wrong, but
you then make matters worse by assigning to parameters[0]. In the
general case, you count the delimiters but then store a string for every
token (that's one more than the number of delimiters). You always
overrun this array.

I suspect you've got confused as to whether you choose to count
delimiters of parameters. Personally, I'd count parameters.

Given that you've already counted either delimiters (or parameters), I
would suggest you make it clear by making this loop a simple counting
one:

for (int i = 0; i < idx; i++) {
const char *tok = strtok(i == 0 ? saved_token : NULL, " ");
cont int size_t = strlen(tok);
parameters = malloc(len + 1);
// Failure test removed to reduce my typing.
strcpy(parameters, tok);
}

I've also removed the odd call to calloc that allocates 30 times the
storage that's needed. Instead, I allocate only what's needed for each
string and so the copy with a plain strcpy.

if (parameters != NULL){
for (parametercount = idx ; parametercount >= 0; --parametercount)
free(parameters[parametercount]);

Here you free elements that you never set. Even if you used every
available element, the very first free call accesses memory outside the
array.

The elements are assigned above in the code. In for loop.

Not parameters[idx]. Well, maybe I should be more explicit. You
allocate room for idx pointers indexed from 0 to idx-1, so either the
first loop will overrun the array (by assigning to parameters[idx]), or,
when that's fixed, this loop frees a pointer that was never set.

Let me know if you find anything else suspicious which can resolve
this problem.

If you worked on making the code clearer, i.e. writing just what you
mean, I think you'd have been able to find most of these yourself.

A tip: can you run the code under a memory checker like valgrind? It's
found more bugs in my code than any other debug tool. I use it whenever
I can.

 

[Keith Thompson]

The fact that the sizeof *ptr method is robust to the type of ptr changing is
a real advantage.

I'm glad you realize that.

That has to be balanced against the disadvantage of writing
code which is difficult for a human to read.

No, it doesn't. Learn the idiom once, understand it forever.

Do *you* find it difficult, or do you merely assume that everyone
else does? (I've asked you this, in different words, several times;
you have yet to answer.)

If your goal is to write code that can be understood by
non-programmers, or by programmers unfamiliar with the language
it's written in, I suggest C is not the language for you. Cobol was
meant to achieve that goal (no comment on whether it succeeded).

[...]

 

[Keith Thompson]

On 08/24/2013 03:05 PM, Sharwan Joram wrote: [...]

if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

fprintf(stdout,"CLI: Couldn't allocate sufficient memory. Exiting \n");
exit(1);
}
strncpy(parameters[parametercount], token, strlen(token));

strlen() has to examine every character in the string, to look for the
terminating null character. strncpy() needs to retrieve the value of
every character in the string. If you'd simply called strcpy() instead,
then it would only have gone through the string once.

In some contexts, strncpy(dest, src, count) can be safer than
strcpy(dest,src). That's true only when "count" is less than strlen(src)
and also less than the length of the array pointed at by dest.
strncpy(dest, src, strlen(src)) stops at exactly the same place that
strcpy() would stop - you don't get any protection at all. If
strlen(src) is bigger than the size of the array pointed at by dest,
strncpy(dest,src,strlen(src)) will still overwrite then end of that
array, just like strcpy().

[...]

You missed the most dangerous thing about strncpy(): it doesn't
necessarily null-terminate the target array (and in this case it won't).
It's *not* just a safer strcpy().

http://the-flat-trantor-society.blogspot.com/2012/03/no-strncpy-is-not-safer-strcpy.html

 

[James Kuyper]

On 08/24/2013 03:05 PM, Sharwan Joram wrote: [...]

if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

fprintf(stdout,"CLI: Couldn't allocate sufficient memory. Exiting \n");
exit(1);
}
strncpy(parameters[parametercount], token, strlen(token));

strlen() has to examine every character in the string, to look for the
terminating null character. strncpy() needs to retrieve the value of
every character in the string. If you'd simply called strcpy() instead,
then it would only have gone through the string once.

In some contexts, strncpy(dest, src, count) can be safer than
strcpy(dest,src). That's true only when "count" is less than strlen(src)
and also less than the length of the array pointed at by dest.
strncpy(dest, src, strlen(src)) stops at exactly the same place that
strcpy() would stop - you don't get any protection at all. If
strlen(src) is bigger than the size of the array pointed at by dest,
strncpy(dest,src,strlen(src)) will still overwrite then end of that
array, just like strcpy().

[...]

You missed the most dangerous thing about strncpy(): it doesn't
necessarily null-terminate the target array (and in this case it won't).
It's *not* just a safer strcpy().

You're right - I deliberately didn't comment on that issue in his
original code, which always memset() the first 30 bytes. As a result, if
code which used those parameters was always careful to read either until
the 30th byte had been read, or until a null character was read,
whichever comes first, the failure to null-terminate the parameters
could have been correct (though I think it unlikely, particularly in the
context of the other errors in his code).

However, when he followed my suggestion to remove the memset(), but
failed to follow my suggestion to change strncpy() to strcpy(), that
removed any possibility that the rest of his code could deal with
parameters stored in this fashion. I should have reconsidered the issue.

 

[gdotone]

Hi,

I have a strange problem here in my code. I'am getting memory corruption while trying to free the first element of list. Code snippet and GDB debugging trace below :



---------------------------------- code

char **parameters;

int idx;

int parametercount;

char *saved_token, token;



parameters = (char **)malloc(parametercount * sizeof(char *)); // Don't use *parameters it breaks.

for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

parameters[parametercount] = (char *)malloc(30 * sizeof (char *));

memset(parameters[parametercount], '\0', 30);

memcpy(parameters[parametercount], token, strlen(token));

parameters[parametercount] = token;

}



/* idx contains the number of tokens */

if (parameters != NULL){

for (parametercount = idx; parametercount >= 0; ++parametercount)

free(parameters[parametercount]);

free(parameters);

}



---------- Debugging session trace ----



160 memcpy(temp_token, saved_token, strlen(saved_token));

(gdb)

161 idx = parametercount = detect_delim_count(temp_token, delimiters);

(gdb)

162 if (temp_token)

(gdb)

163 free(temp_token);

(gdb) n

171 parameters = (char **)malloc(parametercount * sizeof(char *)); // Don't use *parameters it breaks.

(gdb)

172 for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

(gdb)

173 parameters[parametercount] = (char *)malloc(30 * sizeof (char *));

(gdb) p parameters

$1 = (char **) 0xb6e0f828

(gdb) n

174 memset(parameters[parametercount], '\0', 30);

(gdb)

175 memcpy(parameters[parametercount], token, strlen(token));

(gdb)

172 for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

(gdb)

173 parameters[parametercount] = (char *)malloc(30 * sizeof (char *));

(gdb) p parameters

$2 = (char **) 0xb6e0f828

(gdb) n

174 memset(parameters[parametercount], '\0', 30);

(gdb) p parameters[parametercount]

$3 = 0xb6e0f8b8 ""

(gdb) n

175 memcpy(parameters[parametercount], token, strlen(token));

(gdb)

172 for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

(gdb) p parameters[parametercount]

$4 = 0xb6e0f8b8 "param2"

(gdb) n

173 parameters[parametercount] = (char *)malloc(30 * sizeof (char *));

(gdb)

174 memset(parameters[parametercount], '\0', 30);

(gdb) p parameters[parametercount]

$5 = 0xb6e0f938 ""

(gdb)

$6 = 0xb6e0f938 ""

(gdb) n

175 memcpy(parameters[parametercount], token, strlen(token));

(gdb) p parameters[parametercount]

$7 = 0xb6e0f938 ""

(gdb) n

172 for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

(gdb) p parameters[parametercount]

$8 = 0xb6e0f938 "param3"

(gdb) n

173 parameters[parametercount] = (char *)malloc(30 * sizeof (char *));

(gdb)

174 memset(parameters[parametercount], '\0', 30);

(gdb)

175 memcpy(parameters[parametercount], token, strlen(token));

(gdb)

172 for( parametercount = 0 , token = strtok(saved_token, " "); token && *token ; ++parametercount , token = strtok(NULL, " ")){

(gdb) p parameters[parametercount]

$9 = 0xb6e0f9b8 "param4"

(gdb) n

201 shutdown = (currentcommand->command_handler)(client_fd, parameters, parametercount);

(gdb)

211 executedcommand = currentcommand;

(gdb)

216 currentcommand = currentcommand->next;

(gdb)

127 while((executedcommand == NULL) && (currentcommand != NULL)){

(gdb)

221 if (parameters != NULL){

(gdb)

222 for (parametercount = idx; parametercount >= 0; ++parametercount)

(gdb) p parameters

$10 = (char **) 0xb6e0f828

(gdb) p parameters[parametercount]

$11 = 0x61726170 <Address 0x61726170 out of bounds>

(gdb) n

223 free(parameters[parametercount]);

(gdb) p parameters[parametercount]

$12 = 0xb6e0f9b8 "param4"

(gdb) n

*** glibc detected *** /home/sources/opennop-daemon/opennopd/opennopd: corrupted double-linked list: 0xb6e0f820 ***

======= Backtrace: =========

/lib/libc.so.6[0x4441a1d1]

/lib/libc.so.6[0x4441a7bd]

/home/sources/opennop-daemon/opennopd/opennopd[0x805294c]

/home/sources/opennop-daemon/opennopd/opennopd[0x80520c5]

/lib/libpthread.so.0[0x4455fadf]

/lib/libc.so.6(clone+0x5e)[0x4449944e]

======= Memory map: ========

08048000-08056000 r-xp 00000000 fd:01 397476 /home/sources/opennop-daemon/opennopd/opennopd

08056000-08057000 rw-p 0000d000 fd:01 397476 /home/sources/opennop-daemon/opennopd/opennopd

08057000-082ce000 rw-p 00000000 00:00 0 [heap]

44382000-443a1000 r-xp 00000000 fd:01 148916 /usr/lib/ld-2.15.so

443a1000-443a2000 r--p 0001e000 fd:01 148916 /usr/lib/ld-2.15.so

443a2000-443a3000 rw-p 0001f000 fd:01 148916 /usr/lib/ld-2.15.so

443a5000-44550000 r-xp 00000000 fd:01 148917 /usr/lib/libc-2.15.so

44550000-44551000 ---p 001ab000 fd:01 148917 /usr/lib/libc-2.15.so

44551000-44553000 r--p 001ab000 fd:01 148917 /usr/lib/libc-2.15.so

44553000-44554000 rw-p 001ad000 fd:01 148917 /usr/lib/libc-2.15.so

44554000-44557000 rw-p 00000000 00:00 0

44559000-4456f000 r-xp 00000000 fd:01 148918 /usr/lib/libpthread-2.15.so

4456f000-44570000 r--p 00015000 fd:01 148918 /usr/lib/libpthread-2.15.so

44570000-44571000 rw-p 00016000 fd:01 148918 /usr/lib/libpthread-2.15.so

44571000-44573000 rw-p 00000000 00:00 0

44575000-44578000 r-xp 00000000 fd:01 148924 /usr/lib/libdl-2.15.so

44578000-44579000 r--p 00002000 fd:01 148924 /usr/lib/libdl-2.15.so

44579000-4457a000 rw-p 00003000 fd:01 148924 /usr/lib/libdl-2.15.so

448f5000-44911000 r-xp 00000000 fd:01 148934 /usr/lib/libgcc_s-4.7.2-20120921.so.1

44911000-44912000 rw-p 0001b000 fd:01 148934 /usr/lib/libgcc_s-4.7.2-20120921.so.1

45642000-45692000 r-xp 00000000 fd:01 148937 /usr/lib/libfreebl3.so

45692000-45693000 rw-p 00050000 fd:01 148937 /usr/lib/libfreebl3.so

45693000-45697000 rw-p 00000000 00:00 0

456a8000-456b0000 r-xp 00000000 fd:01 148938 /usr/lib/libcrypt-2.15.so

456b0000-456b1000 r--p 00007000 fd:01 148938 /usr/lib/libcrypt-2.15.so

456b1000-456b2000 rw-p 00008000 fd:01 148938 /usr/lib/libcrypt-2.15.so

456b2000-456d9000 rw-p 00000000 00:00 0

b21ff000-b2200000 ---p 00000000 00:00 0

b2200000-b2a00000 rw-p 00000000 00:00 0 [stack:846]

b2a00000-b2b00000 rw-p 00000000 00:00 0

b2bf9000-b2bfa000 ---p 00000000 00:00 0

b2bfa000-b33fa000 rw-p 00000000 00:00 0 [stack:844]

b33fa000-b33fb000 ---p 00000000 00:00 0

b33fb000-b3bfb000 rw-p 00000000 00:00 0 [stack:843]

b3bfb000-b3bfc000 ---p 00000000 00:00 0

b3bfc000-b43fc000 rw-p 00000000 00:00 0 [stack:842]

b43fc000-b43fd000 ---p 00000000 00:00 0

b43fd000-b4bfd000 rw-p 00000000 00:00 0 [stack:840]

b4bfd000-b4bfe000 ---p 00000000 00:00 0

b4bfe000-b53fe000 rw-p 00000000 00:00 0 [stack:836]

b53fe000-b53ff000 ---p 00000000 00:00 0

b53ff000-b5bff000 rw-p 00000000 00:00 0 [stack:835]

b5bff000-b5c00000 ---p 00000000 00:00 0

b5c00000-b6400000 rw-p 00000000 00:00 0 [stack:834]

b6400000-b6500000 rw-p 00000000 00:00 0

b65ff000-b6600000 ---p 00000000 00:00 0

b6600000-b6e00000 rw-p 00000000 00:00 0 [stack:833]

b6e00000-b6e2a000 rw-p 00000000 00:00 0

b6e2a000-b6f00000 ---p 00000000 00:00 0

b6fd2000-b6fd3000 ---p 00000000 00:00 0

b6fd3000-b77d3000 rw-p 00000000 00:00 0 [stack:832]

b77d3000-b77d4000 ---p 00000000 00:00 0

b77d4000-b7fd6000 rw-p 00000000 00:00 0 [stack:831]

b7fd6000-b7fda000 r-xp 00000000 fd:01 165010 /usr/lib/libmnl.so.0.1.0

b7fda000-b7fdb000 r--p 00003000 fd:01 165010 /usr/lib/libmnl.so.0.1.0

b7fdb000-b7fdc000 rw-p 00004000 fd:01 165010 /usr/lib/libmnl.so.0.1.0

b7fdc000-b7fe2000 r-xp 00000000 fd:01 165001 /usr/lib/libnfnetlink.so.0.2.0

b7fe2000-b7fe3000 r--p 00005000 fd:01 165001 /usr/lib/libnfnetlink.so.0.2.0

b7fe3000-b7fe4000 rw-p 00006000 fd:01 165001 /usr/lib/libnfnetlink.so.0.2.0

b7fe4000-b7fe5000 rw-p 00000000 00:00 0

b7fe5000-b7feb000 r-xp 00000000 fd:01 148876 /usr/lib/libnetfilter_queue.so.1.3.0

b7feb000-b7fec000 r--p 00005000 fd:01 148876 /usr/lib/libnetfilter_queue.so.1.3.0

b7fec000-b7fed000 rw-p 00006000 fd:01 148876 /usr/lib/libnetfilter_queue.so.1.3.0

b7ffc000-b7fff000 rw-p 00000000 00:00 0

b7fff000-b8000000 r-xp 00000000 00:00 0 [vdso]

bffdf000-c0000000 rw-p 00000000 00:00 0 [stack]



I am unable to understand the reason of this behaviour.



--Regards,

Sharwan Joram

/* small steps may help */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAXNUMBER 5

int main( void )
{

char **a; /* a pointer to a pointer of type char */


int numberOfPointers = 5;

/* a points to an allocated block of memory which are pointers */
/* in this case 5 such pointer. */

a = (char **) malloc ( numberOfPointers * ( sizeof(char * ) ) );


/* check to see if space was allocated */
/* and print a message if it was */

if ( a!= NULL )
printf( "the array, a, points to declare space\n\n" );


/* declare and iniitialized an array of pointers to characters */

char *b[] = { "one", "two", "three", "four", "five" };


/* pointer assignment */

a = b;


/*
using the pointer to pointer of type char, a, print
the strings of character array b.
*/

for ( int index = 0; index < 5; index++ )
{
printf( "%s\n\n", *(a+index) );
}


/*
find the size of a sting of characters
*/


char *string = "word" ;

size_t n = strlen( string );

printf("\n %zd \n\n", n); /* %zd because n is of type size_t */


/*
declare an array of characters, sentence[]
declare a token, which is a pointer to a string constant, " ".
declare a pointer to a character.

*/


char sentence[] = "this is a sentence with words";
char *token = " ";
char *g;

/*
grab the first word of the character array sentence.

char *strtok( char *s1, const char *s2 );
strtok needs an array of character passed as char *s1 on my machine. when a pointer to a string constant is use
a memory problem occurs.

get the length of the first word, after a call to strtok().
size_t strlen( const char *s );
print the length of that string, the first word in the arrary of characters, with a message.
*/

g = strtok(sentence, token);

size_t y = strlen(g);

printf("the length of the first word in the sentence is %d\n\n", (int) y );

/*
using the length of the string, of the first word, from the char sentence array,
char sentence[], allocate some space that will hold characters plus one more. assign a pointer,
c_space, char *c_space, to point to that created space.

print a message if space was allocated.
*/


char *c_space = (char *) malloc( y * sizeof(char) + 1 );

if (c_space)
{
printf("got space for word\n\n");
}



/* create and array using the length of the first word plus one copy */

int indexnewstring = (int) y + 1; /* (int) strlen( g ) + 1 */

char newstring[indexnewstring];


/* copy the first word into newstring and print string */

char *v = strcpy( newstring, g);
printf(" %s\n", v );


/*
create a second sentence array, char sentence2[], and through the sentence using a token to
separate each word.

*/
char sentence2[] = "this is a sentence with words";
g = strtok( sentence2, token );


while ( g != NULL )
{
printf("\n %s", g );
g = strtok( NULL, token );

}

printf("\n");

char sentence3[] = "C programming is fun execpt for pointers, actually it's the most fun";
for( g = strtok(sentence3, token ); g != NULL; g = strtok( NULL, token ) )
printf (" %s\n", g );




return 0;
}

 

[Keith Thompson]

(e-mail address removed) writes:
[huge snip]

Your articles are difficult to read, partly because you're using the
broken Google Groups interface to Usenet.

GG encourages (or at least fails to discourage) very long lines, and it
handles quoted text poorly. Due, I think, to an assumption that
newlines denote paragraph breaks rather than line breaks, quoted text
tends to be double-spaced -- and quoted quoted text tends to be
quadruple-spaced, and so on.

It's better to keep your physical lines of text below 80 columns,
preferably about 72 to allow for insertion of "> > " in followups. And
if you can copy-and-paste an article into an editor, clean up the
double-spacing, and then copy-and-paste it back into your web browser
before posting, that would also be helpful.

And this is more of a style point, but in the article to which I'm now
replying you have a lot of C code (not quoted from a previous article)
with IMHO excessive blank lines. Some blank lines to show structure are
good, but too many just make it difficult to see more than a fraction of
the code on one screen.

Consider using an actual Usenet server rather than using Google Groups.
I use news.eternal-september.org, which is both good and free. You'd
need a Usenet client program. Mozilla Thunderbird works reasonably
well. I use Gnus under Emacs, which you'll like if an only if you like
Emacs.

 

[Malcolm McLean]

No, it doesn't. Learn the idiom once, understand it forever.

Do *you* find it difficult, or do you merely assume that everyone
else does? (I've asked you this, in different words, several times;
you have yet to answer.)

Hard to read. Not difficult intellectually of course.

one issue is

ptr = malloc(N * sizeof *ptr);

uses the same operator glyph in different contexts which aren't lexically
distinguished, except by an optional whitespace convention.

 

[gdotone]

(e-mail address removed) writes:

[huge snip]



Your articles are difficult to read, partly because you're using the

broken Google Groups interface to Usenet.



GG encourages (or at least fails to discourage) very long lines, and it

handles quoted text poorly. Due, I think, to an assumption that

newlines denote paragraph breaks rather than line breaks, quoted text

tends to be double-spaced -- and quoted quoted text tends to be

quadruple-spaced, and so on.



It's better to keep your physical lines of text below 80 columns,

preferably about 72 to allow for insertion of "> > " in followups. And

if you can copy-and-paste an article into an editor, clean up the

double-spacing, and then copy-and-paste it back into your web browser

before posting, that would also be helpful.



And this is more of a style point, but in the article to which I'm now

replying you have a lot of C code (not quoted from a previous article)

with IMHO excessive blank lines. Some blank lines to show structure are

good, but too many just make it difficult to see more than a fraction of

the code on one screen.



Consider using an actual Usenet server rather than using Google Groups.

I use news.eternal-september.org, which is both good and free. You'd

need a Usenet client program. Mozilla Thunderbird works reasonably

well. I use Gnus under Emacs, which you'll like if an only if you like

Emacs.



--

Keith Thompson (The_Other_Keith) (e-mail address removed) <http://www.ghoti.net/~kst>

Working, but not speaking, for JetHead Development, Inc.

"We must do something. This is something. Therefore, we must do this."

-- Antony Jay and Jonathan Lynn, "Yes Minister"

actually i'm just cutting and pasting because i have noticed my typos of the pass.

style, i can certainly tighten things up and drop much of the comments to help
those that are use a tighter style read it. that copying and pasting maybe adding a little
extra. i checked my editor, i thought it was set to 80 chars, it was not, i changed it to
72. i believe the code is less than 80 chars in width. comments are long winded and longer.

hey, thanks i will repost. let know if it better.