Re: [Linuxptp-devel] [PATCH RFC 17/30] Convert call sites to the proper method for setting the time stamping label.

2020-02-18 Thread Jacob Keller



On 2/11/2020 6:04 AM, Richard Cochran wrote:
> Signed-off-by: Richard Cochran > ---
>  clock.c | 6 --
>  nsm.c   | 5 -
>  port.c  | 2 +-
>  rtnl.c  | 2 +-
>  rtnl.h  | 4 +++-
>  5 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/clock.c b/clock.c
> index 159fcb2..5001e66 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -846,6 +846,7 @@ struct clock *clock_create(enum clock_type type, struct 
> config *config,
>  const char *phc_device)
>  {
>   enum servo_type servo = config_get_int(config, NULL, "clock_servo");
> + char ts_label[IF_NAMESIZE], phc[32], *tmp;
>   enum timestamp_type timestamping;
>   int fadj = 0, max_adj = 0, sw_ts;
>   int phc_index, required_modes = 0;
> @@ -853,7 +854,6 @@ struct clock *clock_create(enum clock_type type, struct 
> config *config,
>   const char *uds_ifname;
>   struct port *p;
>   unsigned char oui[OUI_LEN];
> - char phc[32], *tmp;
>   struct interface *iface, *udsif = >uds_interface;
>   struct timespec ts;
>   int sfl;
> @@ -951,7 +951,9 @@ struct clock *clock_create(enum clock_type type, struct 
> config *config,
>   c->timestamping = timestamping;
>   required_modes = clock_required_modes(c);
>   STAILQ_FOREACH(iface, >interfaces, list) {
> - rtnl_get_ts_device(interface_name(iface), iface->ts_label);
> + memset(ts_label, 0, sizeof(ts_label));
> + rtnl_get_ts_device(interface_name(iface), ts_label);
> + interface_set_label(iface, ts_label);

Hmm.. odd that we now add a memset vs the previous  that didn't use one?
What was the reasoning..? In either case unless rtnl_get_ts_device
doesn't null terminate its output, we should be safe in using strncpy
since it would stop at the NULL terminator or the maximum size.

Thanks,
Jake

>   interface_ensure_tslabel(iface);
>   interface_get_tsinfo(iface);
>   if (iface->ts_info.valid &&
> diff --git a/nsm.c b/nsm.c
> index e82fc37..1292c6b 100644
> --- a/nsm.c
> +++ b/nsm.c
> @@ -262,13 +262,16 @@ static void nsm_help(FILE *fp)
>  static int nsm_open(struct nsm *nsm, struct config *cfg)
>  {
>   enum transport_type transport;
> + char ts_label[IF_NAMESIZE];
>   const char *ifname, *name;
>   struct interface *iface;
>   int count = 0;
>  
>   STAILQ_FOREACH(iface, >interfaces, list) {
>   ifname = interface_name(iface);
> - rtnl_get_ts_device(ifname, iface->ts_label);
> + memset(ts_label, 0, sizeof(ts_label));
> + rtnl_get_ts_device(ifname, ts_label);
> + interface_set_label(iface, ts_label);
>   interface_ensure_tslabel(iface);
>   count++;
>   }
> diff --git a/port.c b/port.c
> index 05eb1d6..c20c3fc 100644
> --- a/port.c
> +++ b/port.c
> @@ -2500,7 +2500,7 @@ void port_link_status(void *ctx, int linkup, int 
> ts_index)
>   /* ts_label changed */
>   old_ts_label = interface_label(p->iface);
>   if (if_indextoname(ts_index, ts_label) && strcmp(old_ts_label, 
> ts_label)) {
> - strncpy(p->iface->ts_label, ts_label, MAX_IFNAME_SIZE);
> + interface_set_label(p->iface, ts_label);
>   p->link_status |= TS_LABEL_CHANGED;
>   pr_notice("port %hu: ts label changed to %s", portnum(p), 
> ts_label);
>   }
> diff --git a/rtnl.c b/rtnl.c
> index d9c76d7..b7a2667 100644
> --- a/rtnl.c
> +++ b/rtnl.c
> @@ -87,7 +87,7 @@ static void rtnl_get_ts_device_callback(void *ctx, int 
> linkup, int ts_index)
>   *dst = ts_index;
>  }
>  
> -int rtnl_get_ts_device(const char *device, char *ts_device)
> +int rtnl_get_ts_device(const char *device, char ts_device[IF_NAMESIZE])
>  {
>   int err, fd;
>   int ts_index = -1;
> diff --git a/rtnl.h b/rtnl.h
> index c5ea979..8fef4a9 100644
> --- a/rtnl.h
> +++ b/rtnl.h
> @@ -20,6 +20,8 @@
>  #ifndef HAVE_RTNL_H
>  #define HAVE_RTNL_H
>  
> +#include 
> +
>  typedef void (*rtnl_callback)(void *ctx, int linkup, int ts_index);
>  
>  /**
> @@ -37,7 +39,7 @@ int rtnl_close(int fd);
>   *  at least IF_NAMESIZE bytes long.
>   * @return  Zero on success, or -1 on error.
>   */
> -int rtnl_get_ts_device(const char *device, char *ts_device);
> +int rtnl_get_ts_device(const char *device, char ts_device[IF_NAMESIZE]);
>  
>  /**
>   * Request the link status from the kernel.
> 


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


[Linuxptp-devel] [PATCH RFC 17/30] Convert call sites to the proper method for setting the time stamping label.

2020-02-11 Thread Richard Cochran
Signed-off-by: Richard Cochran 
---
 clock.c | 6 --
 nsm.c   | 5 -
 port.c  | 2 +-
 rtnl.c  | 2 +-
 rtnl.h  | 4 +++-
 5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/clock.c b/clock.c
index 159fcb2..5001e66 100644
--- a/clock.c
+++ b/clock.c
@@ -846,6 +846,7 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
   const char *phc_device)
 {
enum servo_type servo = config_get_int(config, NULL, "clock_servo");
+   char ts_label[IF_NAMESIZE], phc[32], *tmp;
enum timestamp_type timestamping;
int fadj = 0, max_adj = 0, sw_ts;
int phc_index, required_modes = 0;
@@ -853,7 +854,6 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
const char *uds_ifname;
struct port *p;
unsigned char oui[OUI_LEN];
-   char phc[32], *tmp;
struct interface *iface, *udsif = >uds_interface;
struct timespec ts;
int sfl;
@@ -951,7 +951,9 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
c->timestamping = timestamping;
required_modes = clock_required_modes(c);
STAILQ_FOREACH(iface, >interfaces, list) {
-   rtnl_get_ts_device(interface_name(iface), iface->ts_label);
+   memset(ts_label, 0, sizeof(ts_label));
+   rtnl_get_ts_device(interface_name(iface), ts_label);
+   interface_set_label(iface, ts_label);
interface_ensure_tslabel(iface);
interface_get_tsinfo(iface);
if (iface->ts_info.valid &&
diff --git a/nsm.c b/nsm.c
index e82fc37..1292c6b 100644
--- a/nsm.c
+++ b/nsm.c
@@ -262,13 +262,16 @@ static void nsm_help(FILE *fp)
 static int nsm_open(struct nsm *nsm, struct config *cfg)
 {
enum transport_type transport;
+   char ts_label[IF_NAMESIZE];
const char *ifname, *name;
struct interface *iface;
int count = 0;
 
STAILQ_FOREACH(iface, >interfaces, list) {
ifname = interface_name(iface);
-   rtnl_get_ts_device(ifname, iface->ts_label);
+   memset(ts_label, 0, sizeof(ts_label));
+   rtnl_get_ts_device(ifname, ts_label);
+   interface_set_label(iface, ts_label);
interface_ensure_tslabel(iface);
count++;
}
diff --git a/port.c b/port.c
index 05eb1d6..c20c3fc 100644
--- a/port.c
+++ b/port.c
@@ -2500,7 +2500,7 @@ void port_link_status(void *ctx, int linkup, int ts_index)
/* ts_label changed */
old_ts_label = interface_label(p->iface);
if (if_indextoname(ts_index, ts_label) && strcmp(old_ts_label, 
ts_label)) {
-   strncpy(p->iface->ts_label, ts_label, MAX_IFNAME_SIZE);
+   interface_set_label(p->iface, ts_label);
p->link_status |= TS_LABEL_CHANGED;
pr_notice("port %hu: ts label changed to %s", portnum(p), 
ts_label);
}
diff --git a/rtnl.c b/rtnl.c
index d9c76d7..b7a2667 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -87,7 +87,7 @@ static void rtnl_get_ts_device_callback(void *ctx, int 
linkup, int ts_index)
*dst = ts_index;
 }
 
-int rtnl_get_ts_device(const char *device, char *ts_device)
+int rtnl_get_ts_device(const char *device, char ts_device[IF_NAMESIZE])
 {
int err, fd;
int ts_index = -1;
diff --git a/rtnl.h b/rtnl.h
index c5ea979..8fef4a9 100644
--- a/rtnl.h
+++ b/rtnl.h
@@ -20,6 +20,8 @@
 #ifndef HAVE_RTNL_H
 #define HAVE_RTNL_H
 
+#include 
+
 typedef void (*rtnl_callback)(void *ctx, int linkup, int ts_index);
 
 /**
@@ -37,7 +39,7 @@ int rtnl_close(int fd);
  *  at least IF_NAMESIZE bytes long.
  * @return  Zero on success, or -1 on error.
  */
-int rtnl_get_ts_device(const char *device, char *ts_device);
+int rtnl_get_ts_device(const char *device, char ts_device[IF_NAMESIZE]);
 
 /**
  * Request the link status from the kernel.
-- 
2.20.1



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