Re: Queue question wih dyn mem alloc

On Mon, 28 Dec 2009 14:33:01 -0800 (PST), cerr <ron.eggler@xxxxxxxxx>
wrote:

On Dec 28, 1:59 pm, Barry Schwarz <schwa...@xxxxxxxx> wrote:
On Mon, 28 Dec 2009 11:43:28 -0800 (PST), cerr <ron.egg...@xxxxxxxxx>
wrote:

Hi,
I wrote two functions to push() a character array onto an array and
to
pop() the first element[0] of that array again.
The array holding the elements should be dynamically extendible. So
my
this is my code, the push () function seems to work well but i have
questions for my pop() implementation.

snip an obviously incorrect copy of push

int pop (char ** list, char *outstr, int curlen)
/*******************************************************
  -Description-
  This function pops the first value of an array
  carrying character arrays. This function can be used
  with the push function for a FIFO (First In-First Out)
  data structure that stands for the std::queue
  that is availabld in C++.
  -Parameters-
  char **list - char double pointer to the list carrying
                the strings
  char *outstr- char array pointing to a pre allocated
                string where the first value of the array
                will be copied to.
  int len     - integer (global variable from the caller)

Apparently you meant curlen, not len.  

Exactly correct - typo

Furthermore, you have no idea
how the argument corresponding to this parameter is coded in the
calling function.  The assumption that it is global seems particularly
unlikely since, if it were, there would be no need to pass it as an
argument.

Yup, you're right.... I just "meant" that the variable must be
transparent between push() and pop()

                that carries the number of elements that
                list carries
  -Return value-
  integer     - the int value returned by the function
                represents the number of elements in list.
                This will basically be curlen - 1.
*******************************************************/
{
 int j=0;
 int i=0;
 char **temp;
 if (curlen==0) {
   printf("pop(): No element left in list\n");
   outstr="";
   return 0;
 }
/* WARNING!!! Why does it seem to still work fine if there's fewer
bytes allocated to outstr than list[0] is long?*/

Bad karma.  One of the worst manifestations of undefined behavior is
to appear to work as expected.

So...? What do you recommend to do here? Just to ensure outstr is big
enough to be able to hold list{0]'s content?

I know of no way for pop() to be able to determine this on its own.
One possible way to assist pop() would be to pass the size of whatever
outstr points to as an additional parameter. Another option is to
return the address of the popped string (no size issue since it is not
being copied) and let the calling function free it.


 strcpy(outstr, list[0]);
 for (j=1; j<curlen; j++){
   temp = realloc(list,(curlen)*sizeof(*list));

How many times do you think you need to reallocate list?  Hint: how
many items did you pop off the list?

There'll be as many pop as there were pushes - no limit

You missed the point. One call to pop() only pops one item off. You
are reallocating list curlen times each time you call pop(). curlen-1
of those reallocations are a waste.



Did you mean to use curlen-1 here?  The only purpose is to reduce the
amount of dynamically allocated memory that list points to and this
allocates the exact same amount of memory.

Exactly, that's what i thought as well but if I make curlen-1 i always
end up getting a segmentation fault and i can't figure out why... :(

So it is better to camouflage the problem than solve it?!


   if (temp==NULL){
     printf("pop(): Error reallocating temp memory for msglist\n");
     for (i=curlen;i>=0;i--) {
       free(temp[i]);

Since temp is NULL, it should be obvious that temp[i] does not exist
and attempting to evaluate it invokes undefined behavior.

       curlen--;

Why use i at all when curlen works just as well?  As it stands, this
code is useless since curlen is not used after the for loop variable
has been initialized.

hOOps...:$


     }
     free(list);

So free-ing list would be enough to clean up all the memory in case
something goes wrong on realloc?

Not at all. Every pointer that contains the address of reallocated
memory must be freed individually. Your call to free is using the
wrong argument.


     return -1;
   }
   temp[j-1]=malloc (strlen(list[j])+1);
   strcpy(temp[j-1],list[j]);

There is no need to move the strings themselves.  It is sufficient to
copy the pointers in list to temp.

temp[j-1] = list[j];


 }
 list=temp;

You have just created a massive set of memory leaks.  All the dynamic
areas previously pointed to by the members of list are now
inaccessible.

Hm? But my content (to use) is now in temp... how would I get it back
to list?

Copies of your content is now pointed to by the members of temp. All
the original content was pointed to by the members of list and now
those members and the content they pointed to are still allocated and
unreachable.


 return --curlen;

The extra code necessary to perform the side effect of the -- operator
is a waste since curlen will be destroyed as soon as the function
exits.

Exactly but it will still return the value that can be retrieved by
the caller.

So what is wrong with
return curlen-1;
which doesn't waste resources with unneeded side affects.


}

Also, when i call the pop() function as below, what happens if "res"
isn't "long" enough?

The same undefined behavior described above.

char res[1024]={};
int Len;
Len=pop(msglist,res,Len);
Thanks for hints and suggestions!

Cut and paste; don't retype.

huh??

Your code for push() was obviously not what you compiled.



--
Remove del for email

Please don't quote signatures (the "-- " and subsequent text).

--
Remove del for email
.

Relevant Pages

  • Re: Queue question wih dyn mem alloc
    ... I wrote two functions to pusha character array onto an array and ... the push function seems to work well but i have ...   carrying character arrays. ... that's what i thought as well but if I make curlen-1 i always ...
    (comp.lang.c)
  • Re: Queue question wih dyn mem alloc
    ... I wrote two functions to pusha character array onto an array and ... the push function seems to work well but i have ...   carrying character arrays. ...  Every pointer that contains the address of reallocated ...
    (comp.lang.c)
  • Re: Queue question wih dyn mem alloc
    ... I wrote two functions to pusha character array onto an array and ... the push function seems to work well but i have ...   carrying character arrays. ... Did you mean to use curlen-1 here? ...
    (comp.lang.c)
  • Re: Queue question wih dyn mem alloc
    ... I wrote two functions to pusha character array onto an array and ... the push function seems to work well but i have ...   carrying character arrays. ... Did you mean to use curlen-1 here? ...
    (comp.lang.c)
  • Re: Queue question wih dyn mem alloc
    ... I wrote two functions to pusha character array onto an array and ... the push function seems to work well but i have ... int len - integer ... Did you mean to use curlen-1 here? ...
    (comp.lang.c)