problems with dynamic allocation in an ellipses function

H

hyperboogie

Hello all

I'm pretty new to C, so please accept my apologies in advance :)

I'm trying to allocate space for an array of pointers to strings
(which are accepted as ellipses) inside a while loop, and after the
allocation, when i "assert" the allocation, the assertion fails!!!

void printStrings(s1, ...){ //ellipses function
....
....
void *myList;
int count=1;
char *pArr=s1;
va_start(myList, s1);
while( myList ){
pArr = (char *) realloc(pArr, ++count * sizeoff(char*));
assert(pArr);
pArr[count-1] = va_arg(myList, char*);
}

However, if i take the 2 lines of the allocation and assertion out of
he while, it works!!!
i.e. - the following is OK :

void *myList;
int count=1;
char *pArr=s1;
va_start(myList, s1);

pArr = (char *) realloc(pArr, ++count * sizeoff(char*));
assert(pArr); //THIS IS OK

even if i put these two lines in a for loop that run just once, it
fails!!!

int count=1;
char *pArr=s1;
va_start(myList, s1);
for(i =0; i <1; i++){ // LOOP RUNS JUST ONCE
pArr = (char *) realloc(pArr, ++count * sizeoff(char*));
assert(pArr); //ASSERTION FAILS HERE!!!
}

what Am i doing wrong and why is the allocation failing inside the
while/for loops

Thanks a lot
Shiron
 
R

Richard Heathfield

hyperboogie said:
Hello all

I'm pretty new to C, so please accept my apologies in advance :)

I'm trying to allocate space for an array of pointers to strings
(which are accepted as ellipses) inside a while loop, and after the
allocation, when i "assert" the allocation, the assertion fails!!!

void printStrings(s1, ...){ //ellipses function
...
...
void *myList;
int count=1;
char *pArr=s1;
va_start(myList, s1);
while( myList ){
pArr = (char *) realloc(pArr, ++count * sizeoff(char*));

Make sure you have #include <stdlib.h> near the top of your source file,
lose the cast, use a temp, and get sizeof right:

char *tmp = realloc(pArr, ++count * sizeof *pArr);
if(tmp != NULL)
{
pArr = tmp;
assert(pArr);

In C90, assert requires an integer expression:

assert(pArr != NULL);

but this is not a good way to check whether the allocation succeeded, as
it will abort the application on failure if NDEBUG is not defined, and
will ignore the failure otherwise!
 
H

hyperboogie

hyperboogie said:







Make sure you have #include <stdlib.h> near the top of your source file,
lose the cast, use a temp, and get sizeof right:

char *tmp = realloc(pArr, ++count * sizeof *pArr);
if(tmp != NULL)
{
pArr = tmp;


In C90, assert requires an integer expression:

assert(pArr != NULL);

but this is not a good way to check whether the allocation succeeded, as
it will abort the application on failure if NDEBUG is not defined, and
will ignore the failure otherwise!

Sorry - it doesn't work ....
in any case .... why should I lose the cast? realloc returns void
* and pArr is char * , so a cast is required ... or am I wrong?
 
D

Doug

Hello all

I'm pretty new to C, so please accept my apologies in advance :)

I'm trying to allocate space for an array of pointers to strings
(which are accepted as ellipses) inside a while loop, and after the
allocation, when i "assert" the allocation, the assertion fails!!!

void printStrings(s1, ...){ //ellipses function
...
...
void *myList;
int count=1;
char *pArr=s1;
va_start(myList, s1);
while( myList ){
pArr = (char *) realloc(pArr, ++count * sizeoff(char*));


My guess:

You are attempting to realloc the memory pointed to by pArr. The
first time through the loop, pArr == s1.

If you are calling with something like:

printStrings("This is the string which will be pointed to by s1",
etc., etc., ...

....then I think there's a good chance that the string "This is the
string which will be pointed to by s1" will have static storage/be
read-only due to string pooling/etc.* So I don't think that
realloc'ing this memory is valid.

* I've no doubt got the terminology wrong, but give it ten minutes and
you'll soon see a hundred people correct me!

Hope that helps,
Doug
 
D

Doug

in any case .... why should I lose the cast? realloc returns void
* and pArr is char * , so a cast is required ... or am I wrong?- Hide quoted text -

I think that if you include <stdlib.h> then you don't need the cast -
you are permitted to assign void * to any pointer type.

With that in place, the cast then just hides potential problems.

Doug
 
I

Ian Collins

hyperboogie said:
Hello all

I'm pretty new to C, so please accept my apologies in advance :)

I'm trying to allocate space for an array of pointers to strings
(which are accepted as ellipses) inside a while loop, and after the
allocation, when i "assert" the allocation, the assertion fails!!!

void printStrings(s1, ...){ //ellipses function
....
....
void *myList;
int count=1;
char *pArr=s1;

What is s1? If it isn't a char* allocated by malloc, all bets are off.
 
H

hyperboogie

What is s1? If it isn't a char* allocated by malloc, all bets are off.

s1 is just a string. the function was called from main like so:

printStrings("father", "mother", "brother", "sister", NULL);

one correction to what I wrote earlier:
The function is printStrings(char* s1, ...){...}
 
H

hyperboogie

I think that if you include <stdlib.h> then you don't need the cast -
you are permitted to assign void * to any pointer type.

With that in place, the cast then just hides potential problems.

Doug

I have added <stdlib.h> and tried it with and without the cast - no
difference.
 
D

Doug

s1 is just a string. the function was called from main like so:

printStrings("father", "mother", "brother", "sister", NULL);

one correction to what I wrote earlier:
The function is printStrings(char* s1, ...){...}- Hide quoted text -

Well, this is your problem. In your call to printStrings(), "father"
is not a dynamically allocated piece of memory. Thus, you cannot
realloc it.

Doug
 
D

Doug

I have added <stdlib.h> and tried it with and without the cast - no
difference.

The cast here is not your main problem (and it won't actually affect
this exact code). But it's not good style.

Doug
 
D

Default User

hyperboogie wrote:

s1 is just a string. the function was called from main like so:

printStrings("father", "mother", "brother", "sister", NULL);

one correction to what I wrote earlier:
The function is printStrings(char* s1, ...){...}

For the future, post a complete, minimal program that demonstrates the
problem.

That statement contains three conditions:

1. Complete - A program that can be cut and pasted into our compilers
if we choose, but at the very least we know is not missing anything.

2. Minimal - The program is reduced down as much as possible. That
means removing parts where you can to simplify it, but where it still
produces the failing behavior. Often the process will reveal the
section with the bug, but at least you will make the review process
easier.

3. Demonstrates the problem - The code you give us should be able to
produce the exact condition you are experiencing. In addition, you need
to tell us what the program was supposed to do, and what it did
instead. If it is failing to compile, transcribe exactly or
(preferably) cut and paste the compiler messages.



Brian
 
H

hyperboogie

Well, this is your problem. In your call to printStrings(), "father"
is not a dynamically allocated piece of memory. Thus, you cannot
realloc it.

Doug

Thanks a lot Doug
you were right - that was the problem. I also should have declared
pArr as char ** (as apposed to just char *).
everything works fine now :)
in any case, i still don't understand why I should not use a cast.
After all, realloc returns a:
void *
 
I

Ian Collins

*Please* don't quote signatures!
s1 is just a string. the function was called from main like so:

printStrings("father", "mother", "brother", "sister", NULL);
So there's your problem.
one correction to what I wrote earlier:
The function is printStrings(char* s1, ...){...}
That should realy be:

printStrings( const char* s1, ...);

if s1 is a string literal format string.
 
I

Ian Collins

hyperboogie said:
in any case, i still don't understand why I should not use a cast.
After all, realloc returns a:
void *
You don't need a cast to convert between void* to another pointer type,
that's the way the language is defined.
 
C

CBFalconer

hyperboogie said:
.... snip ...

Sorry - it doesn't work ....
in any case .... why should I lose the cast? realloc returns
void * and pArr is char * , so a cast is required ... or am I wrong?

Yes, you are wrong. See the C-faq.
 
F

Fred Kleinschmidt

hyperboogie said:
Thanks a lot Doug
you were right - that was the problem. I also should have declared
pArr as char ** (as apposed to just char *).
everything works fine now :)
in any case, i still don't understand why I should not use a cast.
After all, realloc returns a:
void *

Consider what happens when you compile this on a 64-bit platform that
has 32-bit int.

If you use
char *x = (char *)malloc(n);
and forgot to #include <stdlib.h>, then there is no prototype for malloc
and the compiler assumes that it returns an int. But malloc returns void*,
which is probably 64 bits, not 32 bits. The compiler assumes malloc is
returning only 32 bits, so that's what it does, clipping the rest.
Then you cast it to a pointer, and you are now corrupt - only half of the
address has been stored in x. Bad news.

If you did not cast it, the compiler would warn you that you are trying to
set a pointer to an int. This would be your clue that something is wrong,
and hopefully you would realize the omission of the prototype.
 
C

CBFalconer

hyperboogie said:
.... snip ...

I have added <stdlib.h> and tried it with and without the cast - no
difference.

If you still get error messages either you have other errors, the
compiler is faulty (doubtful) or you are using a C++ compiler
(probable). Make sure the source file extension is (lowercase) .c.
 
B

Barry Schwarz

I have added <stdlib.h> and tried it with and without the cast - no
difference.

It may not be obvious but there is a world of difference. Without
stdlib (or a suitable prototype from some other source) any call to
malloc, with or without the cast, invokes undefined behavior under
C90. Under C99, with or without the cast, it is a constraint
violation requiring a diagnostic. In either case, not something you
want.


Remove del for email
 
D

David Thompson

void printStrings(s1, ...){ //ellipses function

Nit: that punctuation is an ellipsis (i not e) and specifies a
variadic or varargs function.
void *myList;
int count=1;
char *pArr=s1;

As already noted, this is not something you can realloc below, and
it's also not the right type to store a series of pointers (char*) as
you apparently want to. One way to do that would be:
char * * pArr = malloc (sizeof(char*) /* * 1 */);
or the equivalent
char * * pArr = malloc (sizeof *pArr);
which many people here prefer since it automatically adapts to type
changes for you; (either) followed by:
if( pArr == NULL /* or ! pArr */ ) some error handling;
pArr[0] = s1;
va_start(myList, s1);
while( myList ){

This is a totally wrong way to try to find out what was passed. The
va_list type isn't guaranteed to be usable as a predicate at all, and
if it can be on your implementation, it isn't guaranteed to and almost
certainly won't become false. There is no Standard way for the varargs
mechanism to tell the callee how many arguments there are (or their
types) although SOME preStandard versions did. Your callee code must
(be able to) determine that BEFORE invoking va_arg on a nonexistent
argument, which causes Undefined Behavior, which is Bad (tm
Heathfield?). Three common ways to do this are:
#1- pass an explicit count.
#2- pass information that encodes the count and if necessary types,
such as the format strings used by *printf and *scanf.
#3- after the last real/valid argument pass a sentinel value that the
callee can recognize. For a sequence of pointers such as your case, a
null pointer is usually a good sentinel value. In this case the call
should be written as (char*)NULL (or a macro therefor) -- one of the
few places you SHOULD use a cast since a variadic argument is the only
place that will not automatically be converted, other than
unprototyped calls which you normally should not use.
pArr = (char *) realloc(pArr, ++count * sizeoff(char*));
assert(pArr);
pArr[count-1] = va_arg(myList, char*);
}
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top