ret_pktmbuf_pool_init problem with opaque_arg

ret_pktmbuf_pool_init problem with opaque_arg

[Alejandro Lucero]

Inside this function mbuf_data_room_size is set to a default value if
opaque_arg is null and it should be set to the value pointed by opaque_arg
if not null. Current implementation is using not the value but with the
pointer itself. I think this:

        roomsz = (uint16_t)(uintptr_t)opaque_arg;

should be something like this:

        roomsz = *(uint16_t *)opaque_arg;

Re: ret_pktmbuf_pool_init problem with opaque_arg

[Olivier MATZ]

Hi Alejandro,

On 01/09/2015 03:12 PM, Alejandro Lucero wrote:
> Inside this function mbuf_data_room_size is set to a default value if
> opaque_arg is null and it should be set to the value pointed by opaque_arg
> if not null. Current implementation is using not the value but with the
> pointer itself. I think this:
> 
>         roomsz = (uint16_t)(uintptr_t)opaque_arg;
> 
> should be something like this:
> 
>         roomsz = *(uint16_t *)opaque_arg;
> 

In this particular case, the integer value is stored in the pointer
value: the pointer is not used as a pointer but as an integer. I agree
it can be surprising, but I think the code is correct.

Regards,
Olivier

Re: ret_pktmbuf_pool_init problem with opaque_arg

[Alejandro Lucero]

Hi Olivier,



On Fri, Jan 9, 2015 at 2:28 PM, Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:

> Hi Alejandro,
>
> On 01/09/2015 03:12 PM, Alejandro Lucero wrote:
> > Inside this function mbuf_data_room_size is set to a default value if
> > opaque_arg is null and it should be set to the value pointed by
> opaque_arg
> > if not null. Current implementation is using not the value but with the
> > pointer itself. I think this:
> >
> >         roomsz = (uint16_t)(uintptr_t)opaque_arg;
> >
> > should be something like this:
> >
> >         roomsz = *(uint16_t *)opaque_arg;
> >
>
> In this particular case, the integer value is stored in the pointer
> value: the pointer is not used as a pointer but as an integer. I agree
> it can be surprising, but I think the code is correct.
>
>
Likely there is a good reason for doing things this way but I can not see
the point.

And it will confuse the user.

Thanks and Regards


> Regards,
> Olivier
>

Re: ret_pktmbuf_pool_init problem with opaque_arg

[Stephen Hemminger]

On Fri, 9 Jan 2015 14:12:10 +0000
Alejandro Lucero <alejandro.lucero-wFxRvT7yatFl57MIdRCFDg@public.gmane.org> wrote:

> Inside this function mbuf_data_room_size is set to a default value if
> opaque_arg is null and it should be set to the value pointed by opaque_arg
> if not null. Current implementation is using not the value but with the
> pointer itself. I think this:
> 
>         roomsz = (uint16_t)(uintptr_t)opaque_arg;
> 
> should be something like this:
> 
>         roomsz = *(uint16_t *)opaque_arg;

That would require an extra value in persistent (not stack) for the size.
The way that is implemented, the caller can just pass the value by
casting to unsigned long.