[lwip-devel] [patch #10358] dhcp_start : save dhcp memory type flag.

[MinghaoWang]

Follow-up Comment #4, patch #10358 (project lwip):

[comment #3 comment #3:]
> [comment #2 comment #2:]
> > 
> > [comment #1 comment #1:]
> > > [comment #0 original submission:]
> > > > dhcp_start reset all dhcp flags, so a statically allocated struct dhcp
which was inited by dhcp_set_struct will try to release its memory in
dhcp_cleanup, and that is incorrect.
> > > 
> > > dhcp_cleanup() isn't supposed to be called if using dhcp_set_struct().
See the dhcp_cleanup() docs in dhcp.c:
> > > 
> > >  * ATTENTION: Only use this when not using dhcp_set_struct() to allocate
the
> > >  *            struct dhcp since the memory is passed back to the heap.
> > > 
> > 
> > I think maybe the doc of dhcp_cleanup() is outdated, because after version
2.2.0 the dhcp_cleanup() was designed to accept struct dhcp which allocated by
 dhcp_set_struct(), the dhcp_cleanup() can prevent passing external data to
heap by check flag DHCP_FLAG_EXTERNAL_MEM.
> > The problem is this flag will be cleared in dhcp_start(), cause
dhcp_cleanup() pass external memory to heap incorrectly, and my patch is used
to fix this problem.
> > 
> 
> I see what you're saying. I'll suggest changing the comment and fixing the
flag, however.
> 
> Change:

>     /* save dhcp memory type flag, so we can restore it after clear data
structure */
>     dhcp_flags = dhcp->flags & DHCP_FLAG_EXTERNAL_MEM;


> 
> To this:

>     /* save memory type flag so we can restore it after clearing the
structure */
>     dhcp_flags |= DHCP_FLAG_EXTERNAL_MEM;


> 

I agree to change the comment, but I disagree on the change of code.
The dhcp_start() was called in many places, and the struct dhcp may be
staticly allocated or dynamicly allocated from heap.
We shouldn't set flag DHCP_FLAG_EXTERNAL_MEM directly.



    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?10358>

_______________________________________________
Message sent via Savannah
https://savannah.nongnu.org/