[En-Nut-Discussion] net/ipout.c memory leak

[Stefan Hax]

Sorry, the call to (*nif->if_output)() already frees nb_clone in case of 
error.   Although I have not found my memory leak, NutNetBufFree() does 
not look right when dealing with clones. Here is my suggestion:

extending nb_flags in the structure to be a uint16_t instead of a 
uint8_t then:
#define NBAF_DIE	0x0100

... in /net/netbuf.c:

void NutNetBufFree(NETBUF * nb)
{
     if (nb->nb_flags & NBAF_REFCNT) {                   // referecned 
by something??
         nb->nb_flags |= NBAF_DIE;                       // just mark 
this for death

     } else {                                            // no ref... 
start free'ing what is owned
         if (nb->nb_flags & NBAF_DATALINK) {
             if(nb->nb_ref) {
                 if(nb->nb_ref->nb_dl.vp != nb->nb_dl.vp) { 
  // our's to relese?? then relese it.
                     NutHeapFree(nb->nb_dl.vp);
                 }
             } else {
                 NutHeapFree(nb->nb_dl.vp);
             }
         }

         if (nb->nb_flags & NBAF_NETWORK) {
             if(nb->nb_ref) {
                 if(nb->nb_ref->nb_nw.vp != nb->nb_nw.vp) { 
  // our's to relese?? then relese it.
                     NutHeapFree(nb->nb_nw.vp);
                 }
             } else {
                 NutHeapFree(nb->nb_nw.vp);
             }
         }

         if (nb->nb_flags & NBAF_TRANSPORT) {
             if(nb->nb_ref) {
                 if(nb->nb_ref->nb_tp.vp != nb->nb_tp.vp) { 
  // our's to relese?? then relese it.
                     NutHeapFree(nb->nb_tp.vp);
                 }
             } else {
                 NutHeapFree(nb->nb_tp.vp);
             }
         }

         if (nb->nb_flags & NBAF_APPLICATION) {
             if(nb->nb_ref) {
                 if(nb->nb_ref->nb_ap.vp != nb->nb_ap.vp) { 
  // our,s to relese?? then relese it.
                     NutHeapFree(nb->nb_ap.vp);
                 }
             } else {
                 NutHeapFree(nb->nb_ap.vp);
             }
         }

         if (nb->nb_ref) {
             if(nb->nb_ref->nb_flags & NBAF_REFCNT) {            // dec 
parent count of this ref??
                 nb->nb_ref->nb_flags--;
             }

             if(nb->nb_ref->nb_flags & NBAF_DIE) {               // 
parent marked for death?? then free it..
                 NutNetBufFree(nb->nb_ref);
             }
         }

         NutHeapFree(nb);
     }
}

If what is being freed is the 'parent' and their are still references to 
it then only mark for freeing when all clones have been freed. 
Otherwise, this will just release what bufferes were allocated when the 
clone was created and decrement the reference count in the parent then 
free the clone, and finally, the parent is freed once all references are 
gone.

Stefan.

Stefan Hax wrote:
> noticed a small memory leak in ../net/ipout.c during UDP broadcast failures.
> 
> code is:
> NutIpOutput(..)
> {
> ...
>                 if (nif->if_type == IFT_ETHER)
>                      rc = (*nif->if_output) (dev, ETHERTYPE_IP, ha, 
> nb_clone);
>                  else if (nif->if_type == IFT_PPP)
>                      rc = (*nif->if_output) (dev, PPP_IP, 0, nb_clone);
> 
>                  if (rc == 0) {
>                      NutNetBufFree(nb_clone);
>                  }
> ...
> }
> 
> 
> sugested:
> NutIpOutput(..)
> {
> ...
>                 if (nif->if_type == IFT_ETHER)
>                      rc = (*nif->if_output) (dev, ETHERTYPE_IP, ha, 
> nb_clone);
>                  else if (nif->if_type == IFT_PPP)
>                      rc = (*nif->if_output) (dev, PPP_IP, 0, nb_clone);
> 
>                  if(nb_clone) {          /* <-- might be a better choice */
>                      NutNetBufFree(nb_clone);
>                  }
> ...
> }
> 
> 
> Regards, Stefan.
> _______________________________________________
> http://lists.egnite.de/mailman/listinfo/en-nut-discussion
>