On Tue, Mar 15, 2022 at 06:45:50PM EDT, Keller, Jacob E wrote:
>> +
>> +int unicast_client_unicast_master_table_received(struct port *p, struct
>> ptp_message *m)
>> +{
>> + struct unicast_master_address *ucma;
>> +
>> + if (!unicast_client_enabled(p)) {
>> + return 0;
>> + }
>> + STAILQ_FOREACH(ucma, &p->unicast_master_table->addrs, list) {
>> + if (addreq(transport_type(p->trp), &ucma->address, &m-
>> >address)) {
>> + break;
>> + }
>> + }
>
>Does STALQ_FOREACH guarantee that ucma is NULL after exiting?
>
>For code clarity I'd rather have a separate variable set set to 1 when addreq
>returns true. That is far easier to read, since not every for each iterator
>works this way.
>
>> + return ucma ? 1 : 0;
>> +}
Thanks. Fixed in next v3 patch.
>> +
>> diff --git a/unicast_client.h b/unicast_client.h
>> index 16e291f..b24c4eb 100644
>> --- a/unicast_client.h
>> +++ b/unicast_client.h
>> @@ -82,4 +82,15 @@ void unicast_client_state_changed(struct port *p);
>> */
>> int unicast_client_timer(struct port *p);
>>
>> +/**
>> + * Check whether a message was received from an entry in the unicast
>> + * master table.
>> + * @param p The port in question.
>> + * @param m The message in question.
>> + * @return One (1) if the message is from an entry in the unicast
>> + * master table, or zero otherwise.
>> + */
>> +int unicast_client_unicast_master_table_received(struct port *p,
>> + struct ptp_message *m);
>> +
>> #endif
>
>The name of the function doesn't quite capture its meaning to me.
>
>Perhaps something like "unicast_message_is_from_master_table_entry"?
>
>I'm ok with the name if Richard is, but thought I'd voice my bikeshed opinion.
Changed to unicast_client_msg_is_from_master_table_entry() in v3 patch.
The unicast_client prefix is to keep with existing naming in the file.
Thanks,
Vincent
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
