On Tue, Aug 18, 2020 at 02:19:31AM +0300, Vladimir Oltean wrote:
> diff --git a/phc2sys.c b/phc2sys.c
> index a36cbe071d7d..c4d72bd7d17a 100644
> --- a/phc2sys.c
> +++ b/phc2sys.c
> @@ -56,18 +56,13 @@
>  #include "uds.h"
>  #include "util.h"
>  #include "version.h"
> +#include "contain.h"

Preserve alphabetical ordering please.

> -static int get_mgt_id(struct ptp_message *msg)
> -{
> -     struct management_tlv *mgt = (struct management_tlv *) 
> msg->management.suffix;
> -     return mgt->id;
> -}
> -
> -static void *get_mgt_data(struct ptp_message *msg)
> -{
> -     struct management_tlv *mgt = (struct management_tlv *) 
> msg->management.suffix;
> -     return mgt->data;
> -}

Here you moved code but also changed it unnecessarily.  The
re-factoring for code re-use makes sense, but please keep the moved
code identical.  It makes the review much easier.

If you want to change the CodingStyle of the moved functions, please
do that in a second patch.

> -/* Return values:
> - * 1: success
> - * 0: timeout
> - * -1: error reported by the other side
> - * -2: local error, fatal
> - */
> -static int run_pmc(struct phc2sys_private *priv, int timeout, int ds_id,
> -                struct ptp_message **msg)
> -{

For this function and the ones following, the patch combines two or
more changes.  Please break this down into steps that are easy to
review:

1. Introduce struct pmc_node inside of phc2sys.c and convert functions
   from 'struct phc2sys_private' to 'struct pmc_node'.

2. Remove the 'static' keyword from the future global functions'
   signatures, still within phc2sys.c

3. Move the code.

> diff --git a/pmc_common.c b/pmc_common.c
> index f07f6f65568f..89d7f40b84fe 100644
> --- a/pmc_common.c
> +++ b/pmc_common.c
> @@ -22,6 +22,8 @@
>  #include <string.h>
>  #include <sys/types.h>
>  #include <unistd.h>
> +#include <net/if.h>
> +#include <poll.h>

Preserve alphabetical ordering please.

>  #include "notification.h"
>  #include "print.h"

Thanks,
Richard


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

Reply via email to