Hi Erez,

> > +               goto failed;
> > +       }
>
> I think it is better to merge this ioctl and the socket creation with
> sk_get_ts_info().
> No reason for duplication.
> You can use a static function or inline one.

The existing code seems to be more readable. One socket creation during
link events and init time is not much heavy operation.

> +       /* clear data and ensure it is not marked valid */
> > +       memset(if_info, 0, sizeof(struct sk_if_info));
> > +       return -1;
>
> You need to close the socket!

The close call is in place. The code structure has been kept similar
to sk_get_ts_info. We want to follow the same
coding guidelines.

Moreover, the bit period calculation has been updated with the new patch
and will get called during initialization and whenever there is a link
event.


Thanks,
Devasish.


On Wed, 14 Dec 2022 at 09:04, Richard Cochran <richardcoch...@gmail.com>
wrote:

> On Tue, Dec 13, 2022 at 10:39:59AM +0530, Devasish Dey wrote:
>
> > > +struct sk_if_info {
> > > +       bool valid;
> > It is better not to use bool in a structure, as the size is usually
> > int, i.e. 64 bits to hold 1 bit.
>
> Because we don't have huge arrays of these structs, the size isn't
> critical IMO.
>
> > [Devasish]: Earlier we had it as an integer. We can update it to uint8_t
> as
> > well. But Richard suggested updating it to Boolean.
> > So, leaving this to Richard.
>
> I think 'bool' is fine here.
>
> Thanks,
> Richard
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to