Re: Queue question wih dyn mem alloc

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?

 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


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... :(

   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?

     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?

 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.

}

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??


--
Remove del for email

Thanks a lot for your help, I do appreciate you assisting me here a
little!

--
roN

.

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. ... 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. ...  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)