Combine alike functions

This is a discussion on Combine alike functions within the C Programming forums, part of the General Programming Boards category; Below 2 functions are almost the same, both are used to free memory in a linked list. Can they be ...

  1. #1
    Registered User
    Join Date
    Dec 2004
    Posts
    64

    Combine alike functions

    Below 2 functions are almost the same, both are used to free memory in a linked list. Can they be combined into one?

    Code:
    void FreeColList(DFNCOL *tmp)
    {
    	if(tmp->ptr != NULL)
    		FreeColList(tmp->ptr);
    	free(tmp);
    }
    
    void FreeOutputList(DFNOUTPUT *tmp)
    {
    	if(tmp->ptr != NULL)
    		FreeOutputList(tmp->ptr);
    	free(tmp);
    }

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,673
    Well if you separate the idea of "list" from the "data", then you could have a generic list function to free any list.

    Oh, and using recusion here just sucks.

    Try a loop, quicker and no danger of smashing your stack with a really long list.
    Code:
    void FreeOutputList(DFNOUTPUT *tmp)
    {
      while ( tmp != NULL ) {
        DFNOUTPUT *next = tmp->next;
        free ( tmp );
        tmp = next;
      }
    }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  3. #3
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Code:
    void FreeNode( void *tmp, int specifier )
    {
        DFNCOL *colnode = tmp;
        DFNOUTPUT *outnode = tmp;
    
        if( specifier == somethingthatmeansDFNCOL && colnode->ptr )
            FreeNode( colnode->ptr, specifier );
        if( specifier == somethingthatmeansDFNOUTPUT &&outnode->ptr )
            FreeNode( outnode->ptr, specifier );
        free( tmp );
    }
    Or maybe we like something like this...

    Code:
    void FreeNode( void *tmp, int specifier )
    {
        if( tmp )
        {
            DFNCOL *colnode = tmp;
            DFNOUTPUT *outnode = tmp;
    
            FreeNode( specifier == somethingthatmeansDFNCOL && colnode->ptr
                ? colnode->ptr
                : outnode->ptr
                , specifier );
            free( tmp );
        }
    }
    That should do it.

    [edit] Curses, foiled again! [/edit]


    Quzah.
    Hope is the first step on the road to disappointment.

  4. #4
    Registered User
    Join Date
    Dec 2004
    Posts
    64
    Thank both of you.

    Salem, your code seems much more reasonable than mine. I just moved a line out of WHILE loop since I don't think redefining a variable time and again and again is a good idea.
    void FreeColList ( DFNCOL *tmp )
    Code:
    {
    	DFNCOL *next = NULL;
    	while ( tmp != NULL )
    	{
    		next = tmp->ptr;
    		free ( tmp );
    		tmp = next;
    	}
    }
    Actually, MSVC initials new created pointer as NULL for me. Here that I manually do it is just to make sure.

    Quazh, your code seems to work, however I don't use it since I am new to pointer. I'd thought it was a good idea to use recursion but seemingly Salem doesn't agree with me.

  5. #5
    Nonconformist Narf's Avatar
    Join Date
    Aug 2005
    Posts
    174
    I'd thought it was a good idea to use recursion but seemingly Salem doesn't agree with me.
    Most people won't. Recursion should be used sparingly and only when there's a significant improvement over the non-recursive solution. In this case recursion doesn't simplify the solution enough to be worth the cost. The cost is that your list can't be too long or you'll probably get some kind of stack overflow error at runtime. You also end up using more memory and taking more time to create and maintain a separate frame for each recursive call to the function.
    Just because I don't care doesn't mean I don't understand.

  6. #6
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    You'll also have lots of fun with a circular list. Of course, the loop is going to murder your program with a circular list too...


    Quzah.
    Hope is the first step on the road to disappointment.

  7. #7
    Registered User
    Join Date
    Dec 2004
    Posts
    64
    If the use of recursion is so dangerous, do you have any idea to make this done without recursion.

    Explanation:
    I want to display a column on the screen, which will be divided into many block filled with different colors. Since the number of blocks varys from time to time, so I cannot just set a fixed number for it because if the number could be smaller than needed, or too large and waste memory.
    Code:
    void ColInit(DFNCOL *tmp, int m)
    {
      tmp->n           = m;
      tmp->rect.left   = xScreen / 3;
      tmp->rect.right  = xScreen / 3 + COLWIDTH;
      tmp->rect.top    = yScreen - COLMARGIN - (int)((heightRect * (m + 1)) + 0.5);
      tmp->rect.bottom = yScreen - COLMARGIN - (int)(heightRect * m + 0.5);
                                      /* plus 0.5 to round the value */
      m++;
    
      if (m < nRects) {
        tmp->ptr = malloc(sizeof(DFNCOL));
        memset(tmp->ptr, 0, sizeof(DFNCOL));
        ColInit(tmp->ptr, m);
      }
    }
    Last edited by thinhare; 09-10-2005 at 08:42 PM.

  8. #8
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    Don't cast malloc().

    Simple.
    Code:
    void ColInit ( DFNCOL *tmp, int m )
    {
        while(1) {
    	tmp->n = m;
    	tmp->rect.left      = xScreen / 3;
    	tmp->rect.right    = xScreen / 3 + COLWIDTH;
    	tmp->rect.top      = yScreen - COLMARGIN -
                                                ( int ) ( ( heightRect *  ( m + 1 ) ) + 0.5 );
                                                // plus 0.5 to round the value
    	tmp->rect.bottom   = yScreen - COLMARGIN -  ( int ) ( heightRect * m + 0.5 );
    	m++;
    
    	if ( m < nRects )
    	{
    		tmp->ptr =  /*( DFNCOL * )*/malloc ( sizeof ( DFNCOL ) );
    		memset ( tmp->ptr, 0, sizeof ( DFNCOL ) );
    		tmp = tmp->ptr;
    	}
                    else break;
        }
    }
    Something like that. And try not to use tabs, either.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  9. #9
    Nonconformist Narf's Avatar
    Join Date
    Aug 2005
    Posts
    174
    If the use of recursion is so dangerous
    It's not dangerous unless you abuse it to make it so. But recursion isn't always an appropriate solution. Especially with linked lists.
    do you have any idea to make this done without recursion.
    If the last thing you do is make a recursive call, you can easily replace it with a loop. I took some creative liberty with your code:
    Code:
    void ColInit(DFNCOL *tmp, int m) {
      do {
        tmp->n             = m;
        tmp->rect.left     = xScreen / 3;
        tmp->rect.right    = xScreen / 3 + COLWIDTH;
        tmp->rect.top      = yScreen - COLMARGIN -
          (int)((heightRect * (m + 1)) + 0.5);
        tmp->rect.bottom   = yScreen - COLMARGIN - 
          (int)(heightRect * m + 0.5);
    
        if (++m < nRects) {
          tmp->ptr = malloc(sizeof *tmp->ptr);
          // memset() probably isn't a good idea
          memset(tmp->ptr, 0, sizeof *tmp->ptr);
          tmp = tmp->ptr;
        }
      } while (m < nRects);
    }
    I commented memset() instead of removing it because I don't know what the members of DFNCOL are. I'm 99.9% sure that it's dangerous though. First, there may be padding bits that you shouldn't touch in a structure. Second, all bits zero is only sure to be correct on integral types. So if your member variables aren't all integral, the program may not be doing what you think it's doing. For example, if you use memset on a next pointer in a linked list, the result might not be a null pointer. Those are tricky bugs to find.
    Just because I don't care doesn't mean I don't understand.

  10. #10
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    How about calloc()?
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  11. #11
    Registered User
    Join Date
    Dec 2004
    Posts
    64
    Thank you dwks. I am correcting my code. There is not only one place where recursion is being used. I do need to rethink about it.

  12. #12
    Registered User
    Join Date
    Dec 2004
    Posts
    64
    but what do you mean by _not use tabs_?

  13. #13
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Use spaces instead. Easier to read when moving code between editors (or onto a forum).


    Quzah.
    Hope is the first step on the road to disappointment.

  14. #14
    Nonconformist Narf's Avatar
    Join Date
    Aug 2005
    Posts
    174
    How about calloc()?
    Same thing. calloc() is basically malloc() that calls memset(). The problem is that memset() fills the memory with all bits zero. Since a null pointer doesn't necessarily point to address 0x0, and the bit pattern for 0.0 in floating-point might not be all bits zero, you can't expect memset() to work all the time on those types. In fact, you can't expect it to work on anything more complicated than integral types. Fun, huh?
    Just because I don't care doesn't mean I don't understand.

  15. #15
    Registered User
    Join Date
    Dec 2004
    Posts
    64
    To Narf, sorry, I don't get your point. If memset is not needed, why calloc does the same thing?

    Yes. I got it. I replied too quickly prior to reading through the posts.
    Last edited by thinhare; 09-10-2005 at 02:56 PM.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Void Functions Help
    By bethanne41 in forum C++ Programming
    Replies: 1
    Last Post: 05-09-2005, 05:30 PM
  2. Functions and Classes - What did I do wrong?
    By redmage in forum C++ Programming
    Replies: 5
    Last Post: 04-11-2005, 11:50 AM
  3. calling functions within functions
    By edd1986 in forum C Programming
    Replies: 3
    Last Post: 03-29-2005, 02:35 AM
  4. Factory Functions HOWTO
    By GuardianDevil in forum Windows Programming
    Replies: 1
    Last Post: 05-01-2004, 01:41 PM
  5. Shell functions on Win XP
    By geek@02 in forum Windows Programming
    Replies: 6
    Last Post: 04-19-2004, 05:39 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21