On 4/17/2023 3:24 PM, Erez wrote:
> 
> 
> On Mon, 17 Apr 2023 at 15:21, Maciek Machnikowski
> <mac...@machnikowski.net <mailto:mac...@machnikowski.net>> wrote:
> 
> 
> 
>     On 4/14/2023 2:58 PM, Erez wrote:
>     >
>     >
>     > On Tue, 11 Apr 2023 at 10:28, Maciek Machnikowski
>     > <mac...@machnikowski.net <mailto:mac...@machnikowski.net>
>     <mailto:mac...@machnikowski.net <mailto:mac...@machnikowski.net>>>
>     wrote:
>     >
>     >
>     >     On 4/8/2023 5:42 PM, Erez wrote:
>     >     >
>     >     > I think I have it:
>     >     >
>     >     > +               /* Run the callback on signaling messages if
>     >     configured */
>     >     > +               if (res == 0 && node->signaling_cb_ena &&
>     >     > (msg_type(*msg) == SIGNALING)) {
>     >     > +                       res=2;
>     >     > +               }
>     >     >
>     >     >
>     >     > -                   management_tlv_id(*msg) != ds_id) {
>     >     > +                   res == 1 && management_tlv_id(*msg) !=
>     ds_id) {
>     >     >
>     >     > Erez :-)
>     >
>     >     Thanks for the effort.
>     >     I started with a similar solution, but after seeing how
>     unreadable it
>     >     became - I decided to add a new variable to address the
>     readability of
>     >     this part of the code. I don't like abusing res to get fewer
>     lines of
>     >     code :)
>     >
>     >
>     > Reusing 'ret' is not abuse.
>     > As we change code we may broaden existing variables, nothing is wrong
>     > with it.
>     > Thought, you are free to rewrite the code in your way :-)
>     > I only show a simple way to achieve it, not necessarily the best.
>     > I think using boolean  'skip_cb' is not sufficient, as you should not
>     > call 'management_tlv_id' with a signalling message!
> 
>     Hey!
>     The management_tlv_id won't be called on signaling messages, because the
>     callback should always return 1 and cause the if to stop checking
> 
> 
> You should not make this assumption, the user may want to end the agent
> on signaling message as well :-)
> You add a new feature, you should not assume how we use it!
> 
> Erez 

OK Will fix in a new revision.
And if we're at it - is there any use case that sends signaling and
management messages over the same UDS socket? They are separate options
inside the config file, so I'd assume they should be separated.

Thanks,
Maciek


_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to