RE: [lwip-users] Patch for IGMP V1/V2 and raw multicast support

[Christiaan Simons]

Gaël wrote:

> Okay. I'll try to make the modification against the official release
> version 1.1.0.

Or please against the current STABLE-1_1_1 if possible.
There are only minor differences between these,
but it speeds things up when working on the latest version.
(less differences to manually check)

> Do you have any advice to implement a timeout handler?

As reference you could have a look at the
tcp_fasttmr(), tcp_slowtmr(), dhcp_coarse_tmr(),
dhcp_fine_tmr() etc.

I'm not sure, but maybe the igmp_tick() might just
be the right function. All sys_timeout() calls must
be used with great care, there must be an alternative
when the NO_SYS macro is set.

> > Also I miss some return value checks on mem_malloc().
>
> Could you point out to me the various issues you noticed.

E.g. see src/core/ipv4/igmp.c
line 125
      group = mem_malloc (sizeof (struct igmp_group));
"group" isn't checked for NULL return

Also some lookup_group() return values aren't checked.
AFAIK it can return NULL as well, while the lines following
this function call try to use the pointer ->-> crash.

These are just the ones I've spotted after a quick
scan. Maybe there are more.

> > >From my perspective "the spirit of LWIP" is:
> >
> > * Raw-API first, additional layers and API's can be
> > nice but should not collide or invade in the basic API.
> > * Robust, portable code.
>
> Except the issues you noticed, what it's your feeling about the
> implementation of IGMP/Multicast layer I propose, in term of integration
> in LWIP?

I only made a quick scan over the code,
it looks good besides a few small slips of the pen.
I'm not sure if the functionality is efficient and good,
for this I need to dive a bit deeper into your code.

> What do you mean by additional layers? Because I don't see what
> could be qualify as "additional layers" in the code I propose.

Additional to the 'src/core' code,
in other words everything that resides in src/api.

Some might even have an improved (private) socket implementation,
so it would be nice if your code can be re-used for this
situation too.



Bye,

Christiaan Simons

Hardware Designer
Axon Digital Design


http://www.axon.tv