programm stops

[Thapani Sawaengsri]

*The politic to follow is* : each time a call to malloc succeeded (did'nt
returned NULL) to obtain a pointer (elsewhere in the program), you shall
call free to desallocate the memory as soon as you don't need your pointer
anymore (It results that the number of malloc calls should be equal to the
number of free calls in your program). For example, assuming your pointer is
local to a function, yes, you shall allocate and desallocate it in the
function to avoid memory overflow. Be also careful to free only valid
pointers and not null pointers.


BTW, my implementation for inputneu :

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

#define MAX 100

int inputneu(char * inbuffer, char** outputarray)
{
int argczwei = 0;

if (inbuffer==NULL) return 0;

if (outputarray)
{
*outputarray = malloc(MAX*sizeof*outputarray);

if(*outputarray)
{
outputarray[argczwei] = strtok(inbuffer, ";");
++argczwei;
while ((outputarray[argczwei] = strtok(NULL, ";")) != NULL
&& argczwei++ < MAX);

Doesn't this loop fail with outputarray[100] = strtok should the loop get
that far.

outputarray[argczwei] = NULL
}
}

return argczwei;
}


int main(void)
{
char **toks;
char buff[]=";toto;tutu;tata; titi;";

toks = malloc(sizeof*toks);

if (toks!=NULL)
{
nbtoks = inputneu(buff, toks);
while(*toks!=NULL)
puts(*toks++);
free(toks);
}

return 0;
}

I see the deallocation of toks in main but I don't see the deallocation of
the inputneu function allocation.

Thapani

 

[Régis Troadec]

"Thapani Sawaengsri" <[email protected]> a écrit dans le message de

Hi,

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

#define MAX 100

int inputneu(char * inbuffer, char** outputarray)
{
int argczwei = 0;

if (inbuffer==NULL) return 0;

if (outputarray)
{
*outputarray = malloc(MAX*sizeof*outputarray);

if(*outputarray)
{
outputarray[argczwei] = strtok(inbuffer, ";");
++argczwei;
while ((outputarray[argczwei] = strtok(NULL, ";")) != NULL
&& argczwei++ < MAX);

Doesn't this loop fail with outputarray[100] = strtok should the loop get
that far.

Damned I am ! Yes, you're right.

while((outputarray[argczwei++] = strtok(NULL, ";")) != NULL && argczwei <
MAX);

outputarray[argczwei] = NULL
}
}

return argczwei;

return argczwei-1;

}


int main(void)
{
char **toks;
char buff[]=";toto;tutu;tata; titi;";

toks = malloc(sizeof*toks);

if (toks!=NULL)
{
nbtoks = inputneu(buff, toks);
while(*toks!=NULL)
puts(*toks++);
free(toks);
}

return 0;
}

I see the deallocation of toks in main but I don't see the deallocation of
the inputneu function allocation.

Ouch again! Furthermore, it's not a good example to apply the things I said
above concerning malloc and free. Thank you for correcting me.
Correction :

int main(void)
{
int nbtoks, i;
char ** toks;
char buff[]=";toto;tutu;tata; titi;";

toks = malloc(sizeof*toks);

if (toks)
{
nbtoks = inputneu(buff, toks);
printf("Nb tokens : %d\n", nbtoks);
for(i=0; i<nbtoks ; ++i)
{
puts(toks);
}

if (*toks)
free(*toks);

free(toks);
}

return 0;
}

}

Regis

 

[Sam Dennis]

These are not needed. You should operate on the inbuffer itself.

Although the original code does not inspire much confidence in its
creator, I do suspect that he would've done this if it was okay to
modify the input string. (strtok is called on it.)

Calling routine has no way of freeing this local 'buffer'.

It does if the first character is not the separator (or the string
is empty); **outputarray will be equal to buffer.

argczwei = 0;
(*outputarray)[argczwei++] = strtok(buffer, ";");

argczwei = 0;
(*outputarray)[argczwei] = strtok(inbuffer, ";");

No, the original can be assumed to be correct in both of these two
aspects, in the absence of a precise specification.

(*outputarray)[argczwei+1] = NULL;

In the original, this line is completely unnecessary.

Is everyone responding to this thread drunk or somesuch? Unusually
for this group, not one wholly correct answer has yet been posted;
at least, none have arrived here.

Obviously, the malloc(strlen(buf)) thing is probably what's making
trouble for the OP, but this function is, quite simply, by far too
ugly to live. Write a well-specified replacement; in fact, here's
my attempt, although I may've mistaken the intent. (It's probably
best not to try to actually use it; I haven't properly checked its
functionality or Standard-conformance status.)

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

/*
* Construct a null-terminated array of tokens from a given string
* of the form "token1:token2:...\0" where each tokenN is a string
* of characters not in the separator string and ':' is matched by
* any character in the separator string; return a pointer to this
* array, whose memory can be deallocated with free(), as can that
* of the tokens themselves as a unit by passing the first element
* only. Tokens may be empty.
*
* If the input string or the separator is a null pointer, returns
* a null-pointer. This also occurs if memory allocation fails at
* any point, probably due to exceeding either a system-wide limit
* or a process quota.
*
* FIXME: will fail spectaculary if char is a large type.
*/

/* Must be at least 2. */
#define INITIAL_SIZE 16

char **
tokens( const char *input, const char *separators )
{
/* char here is arbitrary; only 0 and 1 are stored */
char sep[CHAR_MAX - CHAR_MIN + 1] = { 1 };
char **a, *p;
size_t i, size = INITIAL_SIZE;

if (!input || !separators)
return NULL;

a = malloc( size * sizeof *a );
if (!a)
return NULL;

*a = malloc( strlen( input ) + 1);
if (!*a) {
free( a );
return NULL;
}

memcpy( *a, input, strlen( input ) + 1 );

if (!*separators) {
a[1] = NULL;
return a;
}

do
sep[*separators++] = 1;
while (*separators);

/*
* Pointless exercise: how can this be changed so that the
* condition is the only way that the loop is ever broken?
* I realised that I should give in when I found the comma
* operator in the condition, which is certainly much more
* evil than almost any use of the break statement.
*
* Of course, there should be no extra variables and not a
* hint of overloading existing ones.
*/
for (i = 1, p = *a; *p; i++) {
if (i + 1 >= size) {
char **new;
size *= 2;
new = realloc( a, size * sizeof *a );
if (!new) {
free( *a );
free( a );
return NULL;
}
a = new;
}

while (!sep[*p])
p++;

if (!*p)
break;

*p++ = 0;
a = p;
}

a = NULL;

return a;
}

 

[David Resnick]

Be also careful to free only valid
pointers and not null pointers.

Minor nitpick: freeing NULL pointers is just fine, always has
been as far as I know.

From N869, with my >> << added
[#2] The free function causes the space pointed to by ptr to
be deallocated, that is, made available for further
allocation. >> If ptr is a null pointer, no action occurs. <<
Otherwise, if the argument does not match a pointer earlier
returned by the calloc, malloc, or realloc function, or if
the space has been deallocated by a call to free or realloc,
the behavior is undefined.

-David

 

[Dan Pop]

Minor nitpick: freeing NULL pointers is just fine, always has
been as far as I know.

Not always, it's a C89 feature. This was a common trap for people using
gcc on SunOS 4. The compiler was C89 conforming and the unsuspecting
programmers assumed full C89 semantics, which was not the case, the
system libraries being pre-ANSI. free(NULL) resulted in a segfault.

Dan

 

[Keith Thompson]

In <[email protected]>


Not always, it's a C89 feature. This was a common trap for people using
gcc on SunOS 4. The compiler was C89 conforming and the unsuspecting
programmers assumed full C89 semantics, which was not the case, the
system libraries being pre-ANSI. free(NULL) resulted in a segfault.

Hmm. I just tried free(NULL) on SunOS 4.1.3 with gcc 2.95.2, and it
worked. But with Sun's pre-C90 compiler, it fails with a segfault.

There are numerous possible explanations for this (and I'm too lazy to
track it down).

 

[Dan Pop]

Hmm. I just tried free(NULL) on SunOS 4.1.3 with gcc 2.95.2, and it
worked. But with Sun's pre-C90 compiler, it fails with a segfault.

There are numerous possible explanations for this (and I'm too lazy to
track it down).

Compiling with gcc -v would probably reveal the answer. Could be that
the machine also had the Sun's C89 implementation installed (invoked by
acc or something like that)? Or that later gcc versions took care of the
issue on SunOS 4?

Dan

 

[Keith Thompson]

Compiling with gcc -v would probably reveal the answer. Could be that
the machine also had the Sun's C89 implementation installed (invoked by
acc or something like that)? Or that later gcc versions took care of the
issue on SunOS 4?

(SunOS 4.1.3 is from around 1989 or so; gcc 2.95.2, which I installed
separately, is from 2001.)

I tried "gcc -v", but the results weren't helpful. The machine
doesn't have Sun's C89 compiler installed. I did see some indications
that it was linking libraries under the gcc installation directory,
but I didn't find a definition of "free" (or "_free") in any of them.

"man free" shows several different versions of the free() function
(declared in <malloc.h>), the default version:

int free(ptr)
char *ptr;

a System V version:

void free(ptr)
void *ptr;

and an XPG2 version:

void free(ptr)
char *ptr;

none of which quite matches either the declaration in
/usr/include/stdlib.h:

extern int free(/* void *ptr */);

or in /usr/include/malloc.h:

typedef char * malloc_t;
extern int free(/* malloc_t ptr */);

(actually, that last one is equivalent to the default version). The
man page doesn't say that any of these can accept a null pointer.

I haven't taken the time to figure out whether these different
versions of free() all exist on the system or how to select which one
I want to use.

It's a big mess, and it makes me very glad this stuff is standardized
now.

 

[Dan Pop]

(SunOS 4.1.3 is from around 1989 or so; gcc 2.95.2, which I installed
separately, is from 2001.)

I tried "gcc -v", but the results weren't helpful. The machine
doesn't have Sun's C89 compiler installed. I did see some indications
that it was linking libraries under the gcc installation directory,
but I didn't find a definition of "free" (or "_free") in any of them.

Another experiment is to compile a program merely containing a free(NULL)
call and look at the generated assembly code to see if the fix was
not generated inline (it should be obvious even without familiarity with
SPARC assembly). If gcc uses its own version of <stdlib.h>, it might
provide the fix by defining free() as a macro, so you may also want to
try (free)(NULL).

And yet another idea is to do an ldd ./a.out on both the crashing
executable generated by cc and the non-crashing one generated by gcc.

Dan

 

[Dave Thompson]

This isn't any safer WRT buffer overflows - the size of buffer is
what's important, not the string length of inbuffer. ITYM
Right.

strncpy( buffer, inbuffer, sizeof buffer );

Obviously, buffer must be an array declared in the scope of this
statement for that to work; I haven't read the preceding discussion,
so I don't know whether this is the case.

It's not, it's a pointer to malloc'ed space -- originally
strlen(inbuffer), corrected by Ioannis to strlen(inbuffer)+1, so
sizeof is completely wrong. strncpy for strlen()+1 is correct, but as
you say no safer than strcpy, and more cluttered.

- David.Thompson1 at worldnet.att.net