bgpd use long long instead of int64_t

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

bgpd use long long instead of int64_t

Claudio Jeker
In some places bgpd just wants something bigger then a 32bit int.
Instead of using int64_t or u_int64_t use (unsigned) long long which is at
least 64bit and therefor good enough. Makes the mess with type definition
of int64_t on various systems go away (including a bunch of type casts).
While there also apply the endian.h cleanup done in bgpd a few days ago.

OK?
--
:wq Claudio

Index: usr.sbin/bgpctl/bgpctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.229
diff -u -p -r1.229 bgpctl.c
--- usr.sbin/bgpctl/bgpctl.c 11 Feb 2019 15:47:55 -0000 1.229
+++ usr.sbin/bgpctl/bgpctl.c 18 Feb 2019 20:53:38 -0000
@@ -23,6 +23,7 @@
 #include <sys/stat.h>
 #include <sys/un.h>
 
+#include <endian.h>
 #include <err.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -84,7 +85,6 @@ void show_attr(void *, u_int16_t, int)
 void show_community(u_char *, u_int16_t);
 void show_large_community(u_char *, u_int16_t);
 void show_ext_community(u_char *, u_int16_t);
-char *fmt_mem(int64_t);
 int show_rib_memory_msg(struct imsg *);
 void send_filterset(struct imsgbuf *, struct filter_set_head *);
 const char *get_errstr(u_int8_t, u_int8_t);
@@ -1761,12 +1761,12 @@ show_ext_community(u_char *data, u_int16
  case EXT_COMMUNITY_TRANS_OPAQUE:
  case EXT_COMMUNITY_TRANS_EVPN:
  memcpy(&ext, data + i, sizeof(ext));
- ext = betoh64(ext) & 0xffffffffffffLL;
- printf("0x%llx", ext);
+ ext = be64toh(ext) & 0xffffffffffffLL;
+ printf("0x%llx", (unsigned long long)ext);
  break;
  case EXT_COMMUNITY_NON_TRANS_OPAQUE:
  memcpy(&ext, data + i, sizeof(ext));
- ext = betoh64(ext) & 0xffffffffffffLL;
+ ext = be64toh(ext) & 0xffffffffffffLL;
  switch (ext) {
  case EXT_COMMUNITY_OVS_VALID:
  printf("valid ");
@@ -1778,26 +1778,26 @@ show_ext_community(u_char *data, u_int16
  printf("invalid ");
  break;
  default:
- printf("0x%llx ", ext);
+ printf("0x%llx ", (unsigned long long)ext);
  break;
  }
  break;
  default:
  memcpy(&ext, data + i, sizeof(ext));
- printf("0x%llx", betoh64(ext));
+ printf("0x%llx", (unsigned long long)be64toh(ext));
  }
  if (i + 8 < len)
  printf(", ");
  }
 }
 
-char *
-fmt_mem(int64_t num)
+static char *
+fmt_mem(long long num)
 {
  static char buf[16];
 
  if (fmt_scaled(num, buf) == -1)
- snprintf(buf, sizeof(buf), "%lldB", (long long)num);
+ snprintf(buf, sizeof(buf), "%lldB", num);
 
  return (buf);
 }
@@ -1822,31 +1822,31 @@ show_rib_memory_msg(struct imsg *imsg)
  continue;
  pts += stats.pt_cnt[i] * pt_sizes[i];
  printf("%10lld %s network entries using %s of memory\n",
-    (long long)stats.pt_cnt[i], aid_vals[i].name,
+    stats.pt_cnt[i], aid_vals[i].name,
     fmt_mem(stats.pt_cnt[i] * pt_sizes[i]));
  }
  printf("%10lld rib entries using %s of memory\n",
-    (long long)stats.rib_cnt, fmt_mem(stats.rib_cnt *
+    stats.rib_cnt, fmt_mem(stats.rib_cnt *
     sizeof(struct rib_entry)));
  printf("%10lld prefix entries using %s of memory\n",
-    (long long)stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
+    stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
     sizeof(struct prefix)));
  printf("%10lld BGP path attribute entries using %s of memory\n",
-    (long long)stats.path_cnt, fmt_mem(stats.path_cnt *
+    stats.path_cnt, fmt_mem(stats.path_cnt *
     sizeof(struct rde_aspath)));
  printf("\t   and holding %lld references\n",
-    (long long)stats.path_refs);
+    stats.path_refs);
  printf("%10lld BGP AS-PATH attribute entries using "
     "%s of memory\n\t   and holding %lld references\n",
-    (long long)stats.aspath_cnt, fmt_mem(stats.aspath_size),
-    (long long)stats.aspath_refs);
+    stats.aspath_cnt, fmt_mem(stats.aspath_size),
+    stats.aspath_refs);
  printf("%10lld BGP attributes entries using %s of memory\n",
-    (long long)stats.attr_cnt, fmt_mem(stats.attr_cnt *
+    stats.attr_cnt, fmt_mem(stats.attr_cnt *
     sizeof(struct attr)));
  printf("\t   and holding %lld references\n",
-    (long long)stats.attr_refs);
+    stats.attr_refs);
  printf("%10lld BGP attributes using %s of memory\n",
-    (long long)stats.attr_dcnt, fmt_mem(stats.attr_data));
+    stats.attr_dcnt, fmt_mem(stats.attr_data));
  printf("%10lld as-set elements in %lld tables using "
     "%s of memory\n", stats.aset_nmemb, stats.aset_cnt,
     fmt_mem(stats.aset_size));
Index: usr.sbin/bgpd/bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.372
diff -u -p -r1.372 bgpd.h
--- usr.sbin/bgpd/bgpd.h 18 Feb 2019 12:35:08 -0000 1.372
+++ usr.sbin/bgpd/bgpd.h 18 Feb 2019 20:53:38 -0000
@@ -1064,33 +1064,33 @@ extern struct rib_names ribnames;
 #define AS_NONE 0
 
 struct rde_memstats {
- int64_t path_cnt;
- int64_t path_refs;
- int64_t prefix_cnt;
- int64_t rib_cnt;
- int64_t pt_cnt[AID_MAX];
- int64_t nexthop_cnt;
- int64_t aspath_cnt;
- int64_t aspath_size;
- int64_t aspath_refs;
- int64_t attr_cnt;
- int64_t attr_refs;
- int64_t attr_data;
- int64_t attr_dcnt;
- int64_t aset_cnt;
- int64_t aset_size;
- int64_t aset_nmemb;
- int64_t pset_cnt;
- int64_t pset_size;
+ long long path_cnt;
+ long long path_refs;
+ long long prefix_cnt;
+ long long rib_cnt;
+ long long pt_cnt[AID_MAX];
+ long long nexthop_cnt;
+ long long aspath_cnt;
+ long long aspath_size;
+ long long aspath_refs;
+ long long attr_cnt;
+ long long attr_refs;
+ long long attr_data;
+ long long attr_dcnt;
+ long long aset_cnt;
+ long long aset_size;
+ long long aset_nmemb;
+ long long pset_cnt;
+ long long pset_size;
 };
 
 struct rde_hashstats {
  char name[16];
- int64_t num;
- int64_t min;
- int64_t max;
- int64_t sum;
- int64_t sumq;
+ long long num;
+ long long min;
+ long long max;
+ long long sum;
+ long long sumq;
 };
 
 #define MRT_FILE_LEN 512
Index: usr.sbin/bgpd/session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
retrieving revision 1.131
diff -u -p -r1.131 session.h
--- usr.sbin/bgpd/session.h 18 Feb 2019 09:58:19 -0000 1.131
+++ usr.sbin/bgpd/session.h 18 Feb 2019 20:53:38 -0000
@@ -149,22 +149,22 @@ struct ctl_conn {
 TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns;
 
 struct peer_stats {
- u_int64_t msg_rcvd_open;
- u_int64_t msg_rcvd_update;
- u_int64_t msg_rcvd_notification;
- u_int64_t msg_rcvd_keepalive;
- u_int64_t msg_rcvd_rrefresh;
- u_int64_t msg_sent_open;
- u_int64_t msg_sent_update;
- u_int64_t msg_sent_notification;
- u_int64_t msg_sent_keepalive;
- u_int64_t msg_sent_rrefresh;
- u_int64_t prefix_rcvd_update;
- u_int64_t prefix_rcvd_withdraw;
- u_int64_t prefix_rcvd_eor;
- u_int64_t prefix_sent_update;
- u_int64_t prefix_sent_withdraw;
- u_int64_t prefix_sent_eor;
+ unsigned long long msg_rcvd_open;
+ unsigned long long msg_rcvd_update;
+ unsigned long long msg_rcvd_notification;
+ unsigned long long msg_rcvd_keepalive;
+ unsigned long long msg_rcvd_rrefresh;
+ unsigned long long msg_sent_open;
+ unsigned long long msg_sent_update;
+ unsigned long long msg_sent_notification;
+ unsigned long long msg_sent_keepalive;
+ unsigned long long msg_sent_rrefresh;
+ unsigned long long prefix_rcvd_update;
+ unsigned long long prefix_rcvd_withdraw;
+ unsigned long long prefix_rcvd_eor;
+ unsigned long long prefix_sent_update;
+ unsigned long long prefix_sent_withdraw;
+ unsigned long long prefix_sent_eor;
  time_t last_updown;
  time_t last_read;
  u_int32_t prefix_cnt;

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Mark Kettenis
> Date: Mon, 18 Feb 2019 21:59:38 +0100
> From: Claudio Jeker <[hidden email]>
>
> In some places bgpd just wants something bigger then a 32bit int.
> Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> least 64bit and therefor good enough. Makes the mess with type definition
> of int64_t on various systems go away (including a bunch of type casts).
> While there also apply the endian.h cleanup done in bgpd a few days ago.
>
> OK?

You could use <stdint.h> and uint64_t instead.  That should be
portable.  But you'd still need to be careful about printf statements
since (u)int64_t might be (unsigned) long on some systems.

Oh, long long doesn't work on MSVC, but you probably don't care about
that.

> --
> :wq Claudio
>
> Index: usr.sbin/bgpctl/bgpctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.229
> diff -u -p -r1.229 bgpctl.c
> --- usr.sbin/bgpctl/bgpctl.c 11 Feb 2019 15:47:55 -0000 1.229
> +++ usr.sbin/bgpctl/bgpctl.c 18 Feb 2019 20:53:38 -0000
> @@ -23,6 +23,7 @@
>  #include <sys/stat.h>
>  #include <sys/un.h>
>  
> +#include <endian.h>
>  #include <err.h>
>  #include <errno.h>
>  #include <fcntl.h>
> @@ -84,7 +85,6 @@ void show_attr(void *, u_int16_t, int)
>  void show_community(u_char *, u_int16_t);
>  void show_large_community(u_char *, u_int16_t);
>  void show_ext_community(u_char *, u_int16_t);
> -char *fmt_mem(int64_t);
>  int show_rib_memory_msg(struct imsg *);
>  void send_filterset(struct imsgbuf *, struct filter_set_head *);
>  const char *get_errstr(u_int8_t, u_int8_t);
> @@ -1761,12 +1761,12 @@ show_ext_community(u_char *data, u_int16
>   case EXT_COMMUNITY_TRANS_OPAQUE:
>   case EXT_COMMUNITY_TRANS_EVPN:
>   memcpy(&ext, data + i, sizeof(ext));
> - ext = betoh64(ext) & 0xffffffffffffLL;
> - printf("0x%llx", ext);
> + ext = be64toh(ext) & 0xffffffffffffLL;
> + printf("0x%llx", (unsigned long long)ext);
>   break;
>   case EXT_COMMUNITY_NON_TRANS_OPAQUE:
>   memcpy(&ext, data + i, sizeof(ext));
> - ext = betoh64(ext) & 0xffffffffffffLL;
> + ext = be64toh(ext) & 0xffffffffffffLL;
>   switch (ext) {
>   case EXT_COMMUNITY_OVS_VALID:
>   printf("valid ");
> @@ -1778,26 +1778,26 @@ show_ext_community(u_char *data, u_int16
>   printf("invalid ");
>   break;
>   default:
> - printf("0x%llx ", ext);
> + printf("0x%llx ", (unsigned long long)ext);
>   break;
>   }
>   break;
>   default:
>   memcpy(&ext, data + i, sizeof(ext));
> - printf("0x%llx", betoh64(ext));
> + printf("0x%llx", (unsigned long long)be64toh(ext));
>   }
>   if (i + 8 < len)
>   printf(", ");
>   }
>  }
>  
> -char *
> -fmt_mem(int64_t num)
> +static char *
> +fmt_mem(long long num)
>  {
>   static char buf[16];
>  
>   if (fmt_scaled(num, buf) == -1)
> - snprintf(buf, sizeof(buf), "%lldB", (long long)num);
> + snprintf(buf, sizeof(buf), "%lldB", num);
>  
>   return (buf);
>  }
> @@ -1822,31 +1822,31 @@ show_rib_memory_msg(struct imsg *imsg)
>   continue;
>   pts += stats.pt_cnt[i] * pt_sizes[i];
>   printf("%10lld %s network entries using %s of memory\n",
> -    (long long)stats.pt_cnt[i], aid_vals[i].name,
> +    stats.pt_cnt[i], aid_vals[i].name,
>      fmt_mem(stats.pt_cnt[i] * pt_sizes[i]));
>   }
>   printf("%10lld rib entries using %s of memory\n",
> -    (long long)stats.rib_cnt, fmt_mem(stats.rib_cnt *
> +    stats.rib_cnt, fmt_mem(stats.rib_cnt *
>      sizeof(struct rib_entry)));
>   printf("%10lld prefix entries using %s of memory\n",
> -    (long long)stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
> +    stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
>      sizeof(struct prefix)));
>   printf("%10lld BGP path attribute entries using %s of memory\n",
> -    (long long)stats.path_cnt, fmt_mem(stats.path_cnt *
> +    stats.path_cnt, fmt_mem(stats.path_cnt *
>      sizeof(struct rde_aspath)));
>   printf("\t   and holding %lld references\n",
> -    (long long)stats.path_refs);
> +    stats.path_refs);
>   printf("%10lld BGP AS-PATH attribute entries using "
>      "%s of memory\n\t   and holding %lld references\n",
> -    (long long)stats.aspath_cnt, fmt_mem(stats.aspath_size),
> -    (long long)stats.aspath_refs);
> +    stats.aspath_cnt, fmt_mem(stats.aspath_size),
> +    stats.aspath_refs);
>   printf("%10lld BGP attributes entries using %s of memory\n",
> -    (long long)stats.attr_cnt, fmt_mem(stats.attr_cnt *
> +    stats.attr_cnt, fmt_mem(stats.attr_cnt *
>      sizeof(struct attr)));
>   printf("\t   and holding %lld references\n",
> -    (long long)stats.attr_refs);
> +    stats.attr_refs);
>   printf("%10lld BGP attributes using %s of memory\n",
> -    (long long)stats.attr_dcnt, fmt_mem(stats.attr_data));
> +    stats.attr_dcnt, fmt_mem(stats.attr_data));
>   printf("%10lld as-set elements in %lld tables using "
>      "%s of memory\n", stats.aset_nmemb, stats.aset_cnt,
>      fmt_mem(stats.aset_size));
> Index: usr.sbin/bgpd/bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.372
> diff -u -p -r1.372 bgpd.h
> --- usr.sbin/bgpd/bgpd.h 18 Feb 2019 12:35:08 -0000 1.372
> +++ usr.sbin/bgpd/bgpd.h 18 Feb 2019 20:53:38 -0000
> @@ -1064,33 +1064,33 @@ extern struct rib_names ribnames;
>  #define AS_NONE 0
>  
>  struct rde_memstats {
> - int64_t path_cnt;
> - int64_t path_refs;
> - int64_t prefix_cnt;
> - int64_t rib_cnt;
> - int64_t pt_cnt[AID_MAX];
> - int64_t nexthop_cnt;
> - int64_t aspath_cnt;
> - int64_t aspath_size;
> - int64_t aspath_refs;
> - int64_t attr_cnt;
> - int64_t attr_refs;
> - int64_t attr_data;
> - int64_t attr_dcnt;
> - int64_t aset_cnt;
> - int64_t aset_size;
> - int64_t aset_nmemb;
> - int64_t pset_cnt;
> - int64_t pset_size;
> + long long path_cnt;
> + long long path_refs;
> + long long prefix_cnt;
> + long long rib_cnt;
> + long long pt_cnt[AID_MAX];
> + long long nexthop_cnt;
> + long long aspath_cnt;
> + long long aspath_size;
> + long long aspath_refs;
> + long long attr_cnt;
> + long long attr_refs;
> + long long attr_data;
> + long long attr_dcnt;
> + long long aset_cnt;
> + long long aset_size;
> + long long aset_nmemb;
> + long long pset_cnt;
> + long long pset_size;
>  };
>  
>  struct rde_hashstats {
>   char name[16];
> - int64_t num;
> - int64_t min;
> - int64_t max;
> - int64_t sum;
> - int64_t sumq;
> + long long num;
> + long long min;
> + long long max;
> + long long sum;
> + long long sumq;
>  };
>  
>  #define MRT_FILE_LEN 512
> Index: usr.sbin/bgpd/session.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
> retrieving revision 1.131
> diff -u -p -r1.131 session.h
> --- usr.sbin/bgpd/session.h 18 Feb 2019 09:58:19 -0000 1.131
> +++ usr.sbin/bgpd/session.h 18 Feb 2019 20:53:38 -0000
> @@ -149,22 +149,22 @@ struct ctl_conn {
>  TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns;
>  
>  struct peer_stats {
> - u_int64_t msg_rcvd_open;
> - u_int64_t msg_rcvd_update;
> - u_int64_t msg_rcvd_notification;
> - u_int64_t msg_rcvd_keepalive;
> - u_int64_t msg_rcvd_rrefresh;
> - u_int64_t msg_sent_open;
> - u_int64_t msg_sent_update;
> - u_int64_t msg_sent_notification;
> - u_int64_t msg_sent_keepalive;
> - u_int64_t msg_sent_rrefresh;
> - u_int64_t prefix_rcvd_update;
> - u_int64_t prefix_rcvd_withdraw;
> - u_int64_t prefix_rcvd_eor;
> - u_int64_t prefix_sent_update;
> - u_int64_t prefix_sent_withdraw;
> - u_int64_t prefix_sent_eor;
> + unsigned long long msg_rcvd_open;
> + unsigned long long msg_rcvd_update;
> + unsigned long long msg_rcvd_notification;
> + unsigned long long msg_rcvd_keepalive;
> + unsigned long long msg_rcvd_rrefresh;
> + unsigned long long msg_sent_open;
> + unsigned long long msg_sent_update;
> + unsigned long long msg_sent_notification;
> + unsigned long long msg_sent_keepalive;
> + unsigned long long msg_sent_rrefresh;
> + unsigned long long prefix_rcvd_update;
> + unsigned long long prefix_rcvd_withdraw;
> + unsigned long long prefix_rcvd_eor;
> + unsigned long long prefix_sent_update;
> + unsigned long long prefix_sent_withdraw;
> + unsigned long long prefix_sent_eor;
>   time_t last_updown;
>   time_t last_read;
>   u_int32_t prefix_cnt;
>
>

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Theo de Raadt-2
Mark Kettenis <[hidden email]> wrote:

> > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > From: Claudio Jeker <[hidden email]>
> >
> > In some places bgpd just wants something bigger then a 32bit int.
> > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > least 64bit and therefor good enough. Makes the mess with type definition
> > of int64_t on various systems go away (including a bunch of type casts).
> > While there also apply the endian.h cleanup done in bgpd a few days ago.
> >
> > OK?
>
> You could use <stdint.h> and uint64_t instead.  That should be
> portable.  But you'd still need to be careful about printf statements
> since (u)int64_t might be (unsigned) long on some systems.

Indeed, that is the problem, the difference between systems where the
64-bit types are "long" vs "long long".

They are both valid, but there is a conflict.

That requires casts to the raw-type in some places, but not others.  It
can be confusing, and as a result people have a tendency to (cautiously)
overcast.

But casting when you shouldn't can be dangerous.  Especially downsize casts.


Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Claudio Jeker
In reply to this post by Mark Kettenis
On Mon, Feb 18, 2019 at 10:11:03PM +0100, Mark Kettenis wrote:

> > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > From: Claudio Jeker <[hidden email]>
> >
> > In some places bgpd just wants something bigger then a 32bit int.
> > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > least 64bit and therefor good enough. Makes the mess with type definition
> > of int64_t on various systems go away (including a bunch of type casts).
> > While there also apply the endian.h cleanup done in bgpd a few days ago.
> >
> > OK?
>
> You could use <stdint.h> and uint64_t instead.  That should be
> portable.  But you'd still need to be careful about printf statements
> since (u)int64_t might be (unsigned) long on some systems.

This issue with int64_t being just a unsigned long on 64bit linux is the
problem I'm trying to avoid since the result is that all printf calls need
casts. long long is %llu on all systems I care and so less ugly in this
specific case. I used uint64_t mostly because it is shorter than
unsigned long long (being lazy is not always a win).
 
> Oh, long long doesn't work on MSVC, but you probably don't care about
> that.

Nope, just BSD and linux (maybe other UNIX variants).
 

> > --
> > :wq Claudio
> >
> > Index: usr.sbin/bgpctl/bgpctl.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> > retrieving revision 1.229
> > diff -u -p -r1.229 bgpctl.c
> > --- usr.sbin/bgpctl/bgpctl.c 11 Feb 2019 15:47:55 -0000 1.229
> > +++ usr.sbin/bgpctl/bgpctl.c 18 Feb 2019 20:53:38 -0000
> > @@ -23,6 +23,7 @@
> >  #include <sys/stat.h>
> >  #include <sys/un.h>
> >  
> > +#include <endian.h>
> >  #include <err.h>
> >  #include <errno.h>
> >  #include <fcntl.h>
> > @@ -84,7 +85,6 @@ void show_attr(void *, u_int16_t, int)
> >  void show_community(u_char *, u_int16_t);
> >  void show_large_community(u_char *, u_int16_t);
> >  void show_ext_community(u_char *, u_int16_t);
> > -char *fmt_mem(int64_t);
> >  int show_rib_memory_msg(struct imsg *);
> >  void send_filterset(struct imsgbuf *, struct filter_set_head *);
> >  const char *get_errstr(u_int8_t, u_int8_t);
> > @@ -1761,12 +1761,12 @@ show_ext_community(u_char *data, u_int16
> >   case EXT_COMMUNITY_TRANS_OPAQUE:
> >   case EXT_COMMUNITY_TRANS_EVPN:
> >   memcpy(&ext, data + i, sizeof(ext));
> > - ext = betoh64(ext) & 0xffffffffffffLL;
> > - printf("0x%llx", ext);
> > + ext = be64toh(ext) & 0xffffffffffffLL;
> > + printf("0x%llx", (unsigned long long)ext);
> >   break;
> >   case EXT_COMMUNITY_NON_TRANS_OPAQUE:
> >   memcpy(&ext, data + i, sizeof(ext));
> > - ext = betoh64(ext) & 0xffffffffffffLL;
> > + ext = be64toh(ext) & 0xffffffffffffLL;
> >   switch (ext) {
> >   case EXT_COMMUNITY_OVS_VALID:
> >   printf("valid ");
> > @@ -1778,26 +1778,26 @@ show_ext_community(u_char *data, u_int16
> >   printf("invalid ");
> >   break;
> >   default:
> > - printf("0x%llx ", ext);
> > + printf("0x%llx ", (unsigned long long)ext);
> >   break;
> >   }
> >   break;
> >   default:
> >   memcpy(&ext, data + i, sizeof(ext));
> > - printf("0x%llx", betoh64(ext));
> > + printf("0x%llx", (unsigned long long)be64toh(ext));
> >   }
> >   if (i + 8 < len)
> >   printf(", ");
> >   }
> >  }
> >  
> > -char *
> > -fmt_mem(int64_t num)
> > +static char *
> > +fmt_mem(long long num)
> >  {
> >   static char buf[16];
> >  
> >   if (fmt_scaled(num, buf) == -1)
> > - snprintf(buf, sizeof(buf), "%lldB", (long long)num);
> > + snprintf(buf, sizeof(buf), "%lldB", num);
> >  
> >   return (buf);
> >  }
> > @@ -1822,31 +1822,31 @@ show_rib_memory_msg(struct imsg *imsg)
> >   continue;
> >   pts += stats.pt_cnt[i] * pt_sizes[i];
> >   printf("%10lld %s network entries using %s of memory\n",
> > -    (long long)stats.pt_cnt[i], aid_vals[i].name,
> > +    stats.pt_cnt[i], aid_vals[i].name,
> >      fmt_mem(stats.pt_cnt[i] * pt_sizes[i]));
> >   }
> >   printf("%10lld rib entries using %s of memory\n",
> > -    (long long)stats.rib_cnt, fmt_mem(stats.rib_cnt *
> > +    stats.rib_cnt, fmt_mem(stats.rib_cnt *
> >      sizeof(struct rib_entry)));
> >   printf("%10lld prefix entries using %s of memory\n",
> > -    (long long)stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
> > +    stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
> >      sizeof(struct prefix)));
> >   printf("%10lld BGP path attribute entries using %s of memory\n",
> > -    (long long)stats.path_cnt, fmt_mem(stats.path_cnt *
> > +    stats.path_cnt, fmt_mem(stats.path_cnt *
> >      sizeof(struct rde_aspath)));
> >   printf("\t   and holding %lld references\n",
> > -    (long long)stats.path_refs);
> > +    stats.path_refs);
> >   printf("%10lld BGP AS-PATH attribute entries using "
> >      "%s of memory\n\t   and holding %lld references\n",
> > -    (long long)stats.aspath_cnt, fmt_mem(stats.aspath_size),
> > -    (long long)stats.aspath_refs);
> > +    stats.aspath_cnt, fmt_mem(stats.aspath_size),
> > +    stats.aspath_refs);
> >   printf("%10lld BGP attributes entries using %s of memory\n",
> > -    (long long)stats.attr_cnt, fmt_mem(stats.attr_cnt *
> > +    stats.attr_cnt, fmt_mem(stats.attr_cnt *
> >      sizeof(struct attr)));
> >   printf("\t   and holding %lld references\n",
> > -    (long long)stats.attr_refs);
> > +    stats.attr_refs);
> >   printf("%10lld BGP attributes using %s of memory\n",
> > -    (long long)stats.attr_dcnt, fmt_mem(stats.attr_data));
> > +    stats.attr_dcnt, fmt_mem(stats.attr_data));
> >   printf("%10lld as-set elements in %lld tables using "
> >      "%s of memory\n", stats.aset_nmemb, stats.aset_cnt,
> >      fmt_mem(stats.aset_size));
> > Index: usr.sbin/bgpd/bgpd.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> > retrieving revision 1.372
> > diff -u -p -r1.372 bgpd.h
> > --- usr.sbin/bgpd/bgpd.h 18 Feb 2019 12:35:08 -0000 1.372
> > +++ usr.sbin/bgpd/bgpd.h 18 Feb 2019 20:53:38 -0000
> > @@ -1064,33 +1064,33 @@ extern struct rib_names ribnames;
> >  #define AS_NONE 0
> >  
> >  struct rde_memstats {
> > - int64_t path_cnt;
> > - int64_t path_refs;
> > - int64_t prefix_cnt;
> > - int64_t rib_cnt;
> > - int64_t pt_cnt[AID_MAX];
> > - int64_t nexthop_cnt;
> > - int64_t aspath_cnt;
> > - int64_t aspath_size;
> > - int64_t aspath_refs;
> > - int64_t attr_cnt;
> > - int64_t attr_refs;
> > - int64_t attr_data;
> > - int64_t attr_dcnt;
> > - int64_t aset_cnt;
> > - int64_t aset_size;
> > - int64_t aset_nmemb;
> > - int64_t pset_cnt;
> > - int64_t pset_size;
> > + long long path_cnt;
> > + long long path_refs;
> > + long long prefix_cnt;
> > + long long rib_cnt;
> > + long long pt_cnt[AID_MAX];
> > + long long nexthop_cnt;
> > + long long aspath_cnt;
> > + long long aspath_size;
> > + long long aspath_refs;
> > + long long attr_cnt;
> > + long long attr_refs;
> > + long long attr_data;
> > + long long attr_dcnt;
> > + long long aset_cnt;
> > + long long aset_size;
> > + long long aset_nmemb;
> > + long long pset_cnt;
> > + long long pset_size;
> >  };
> >  
> >  struct rde_hashstats {
> >   char name[16];
> > - int64_t num;
> > - int64_t min;
> > - int64_t max;
> > - int64_t sum;
> > - int64_t sumq;
> > + long long num;
> > + long long min;
> > + long long max;
> > + long long sum;
> > + long long sumq;
> >  };
> >  
> >  #define MRT_FILE_LEN 512
> > Index: usr.sbin/bgpd/session.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
> > retrieving revision 1.131
> > diff -u -p -r1.131 session.h
> > --- usr.sbin/bgpd/session.h 18 Feb 2019 09:58:19 -0000 1.131
> > +++ usr.sbin/bgpd/session.h 18 Feb 2019 20:53:38 -0000
> > @@ -149,22 +149,22 @@ struct ctl_conn {
> >  TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns;
> >  
> >  struct peer_stats {
> > - u_int64_t msg_rcvd_open;
> > - u_int64_t msg_rcvd_update;
> > - u_int64_t msg_rcvd_notification;
> > - u_int64_t msg_rcvd_keepalive;
> > - u_int64_t msg_rcvd_rrefresh;
> > - u_int64_t msg_sent_open;
> > - u_int64_t msg_sent_update;
> > - u_int64_t msg_sent_notification;
> > - u_int64_t msg_sent_keepalive;
> > - u_int64_t msg_sent_rrefresh;
> > - u_int64_t prefix_rcvd_update;
> > - u_int64_t prefix_rcvd_withdraw;
> > - u_int64_t prefix_rcvd_eor;
> > - u_int64_t prefix_sent_update;
> > - u_int64_t prefix_sent_withdraw;
> > - u_int64_t prefix_sent_eor;
> > + unsigned long long msg_rcvd_open;
> > + unsigned long long msg_rcvd_update;
> > + unsigned long long msg_rcvd_notification;
> > + unsigned long long msg_rcvd_keepalive;
> > + unsigned long long msg_rcvd_rrefresh;
> > + unsigned long long msg_sent_open;
> > + unsigned long long msg_sent_update;
> > + unsigned long long msg_sent_notification;
> > + unsigned long long msg_sent_keepalive;
> > + unsigned long long msg_sent_rrefresh;
> > + unsigned long long prefix_rcvd_update;
> > + unsigned long long prefix_rcvd_withdraw;
> > + unsigned long long prefix_rcvd_eor;
> > + unsigned long long prefix_sent_update;
> > + unsigned long long prefix_sent_withdraw;
> > + unsigned long long prefix_sent_eor;
> >   time_t last_updown;
> >   time_t last_read;
> >   u_int32_t prefix_cnt;
> >
> >
>

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Theo de Raadt-2
Claudio Jeker <[hidden email]> wrote:

> On Mon, Feb 18, 2019 at 10:11:03PM +0100, Mark Kettenis wrote:
> > > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > > From: Claudio Jeker <[hidden email]>
> > >
> > > In some places bgpd just wants something bigger then a 32bit int.
> > > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > > least 64bit and therefor good enough. Makes the mess with type definition
> > > of int64_t on various systems go away (including a bunch of type casts).
> > > While there also apply the endian.h cleanup done in bgpd a few days ago.
> > >
> > > OK?
> >
> > You could use <stdint.h> and uint64_t instead.  That should be
> > portable.  But you'd still need to be careful about printf statements
> > since (u)int64_t might be (unsigned) long on some systems.
>
> This issue with int64_t being just a unsigned long on 64bit

Huh?  Surely you mispoke, it is not unsigned.

 linux is the
> problem I'm trying to avoid since the result is that all printf calls need
> casts. long long is %llu on all systems I care and so less ugly in this
> specific case.

Huh?  Again I'm confused.  long long is %lld, not %llu

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Claudio Jeker
On Mon, Feb 18, 2019 at 02:24:52PM -0700, Theo de Raadt wrote:

> Claudio Jeker <[hidden email]> wrote:
>
> > On Mon, Feb 18, 2019 at 10:11:03PM +0100, Mark Kettenis wrote:
> > > > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > > > From: Claudio Jeker <[hidden email]>
> > > >
> > > > In some places bgpd just wants something bigger then a 32bit int.
> > > > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > > > least 64bit and therefor good enough. Makes the mess with type definition
> > > > of int64_t on various systems go away (including a bunch of type casts).
> > > > While there also apply the endian.h cleanup done in bgpd a few days ago.
> > > >
> > > > OK?
> > >
> > > You could use <stdint.h> and uint64_t instead.  That should be
> > > portable.  But you'd still need to be careful about printf statements
> > > since (u)int64_t might be (unsigned) long on some systems.
> >
> > This issue with int64_t being just a unsigned long on 64bit
>
> Huh?  Surely you mispoke, it is not unsigned.

Arrrgg. int64_t -> long, u_int64_t aka uint64_t -> unsigned long
Had too much type handling today.
 
>  linux is the
> > problem I'm trying to avoid since the result is that all printf calls need
> > casts. long long is %llu on all systems I care and so less ugly in this
> > specific case.
>
> Huh?  Again I'm confused.  long long is %lld, not %llu

Yes, that should have been %lld. Should have payed more attention while
answering.

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Andreas Kusalananda Kähäri-4
In reply to this post by Mark Kettenis
On Mon, Feb 18, 2019 at 10:11:03PM +0100, Mark Kettenis wrote:

> > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > From: Claudio Jeker <[hidden email]>
> >
> > In some places bgpd just wants something bigger then a 32bit int.
> > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > least 64bit and therefor good enough. Makes the mess with type definition
> > of int64_t on various systems go away (including a bunch of type casts).
> > While there also apply the endian.h cleanup done in bgpd a few days ago.
> >
> > OK?
>
> You could use <stdint.h> and uint64_t instead.  That should be
> portable.  But you'd still need to be careful about printf statements
> since (u)int64_t might be (unsigned) long on some systems.

printf should be no issue if you use the correct PRI*64 (PRIu64 or
PRId64) macro from <inttypes.h>.  Both <stdint.h> and <inttypes.h> are
C99.

E.g.

    uint64_t thing;

    /* ... */

    printf("The value is %" PRIu64 "\n", thing);


... but I'm really not qualified to say anything about what you guys should do.

Cheers,


>
> Oh, long long doesn't work on MSVC, but you probably don't care about
> that.
>
> > --
> > :wq Claudio
> >
> > Index: usr.sbin/bgpctl/bgpctl.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> > retrieving revision 1.229
> > diff -u -p -r1.229 bgpctl.c
> > --- usr.sbin/bgpctl/bgpctl.c 11 Feb 2019 15:47:55 -0000 1.229
> > +++ usr.sbin/bgpctl/bgpctl.c 18 Feb 2019 20:53:38 -0000
> > @@ -23,6 +23,7 @@
> >  #include <sys/stat.h>
> >  #include <sys/un.h>
> >  
> > +#include <endian.h>
> >  #include <err.h>
> >  #include <errno.h>
> >  #include <fcntl.h>
> > @@ -84,7 +85,6 @@ void show_attr(void *, u_int16_t, int)
> >  void show_community(u_char *, u_int16_t);
> >  void show_large_community(u_char *, u_int16_t);
> >  void show_ext_community(u_char *, u_int16_t);
> > -char *fmt_mem(int64_t);
> >  int show_rib_memory_msg(struct imsg *);
> >  void send_filterset(struct imsgbuf *, struct filter_set_head *);
> >  const char *get_errstr(u_int8_t, u_int8_t);
> > @@ -1761,12 +1761,12 @@ show_ext_community(u_char *data, u_int16
> >   case EXT_COMMUNITY_TRANS_OPAQUE:
> >   case EXT_COMMUNITY_TRANS_EVPN:
> >   memcpy(&ext, data + i, sizeof(ext));
> > - ext = betoh64(ext) & 0xffffffffffffLL;
> > - printf("0x%llx", ext);
> > + ext = be64toh(ext) & 0xffffffffffffLL;
> > + printf("0x%llx", (unsigned long long)ext);
> >   break;
> >   case EXT_COMMUNITY_NON_TRANS_OPAQUE:
> >   memcpy(&ext, data + i, sizeof(ext));
> > - ext = betoh64(ext) & 0xffffffffffffLL;
> > + ext = be64toh(ext) & 0xffffffffffffLL;
> >   switch (ext) {
> >   case EXT_COMMUNITY_OVS_VALID:
> >   printf("valid ");
> > @@ -1778,26 +1778,26 @@ show_ext_community(u_char *data, u_int16
> >   printf("invalid ");
> >   break;
> >   default:
> > - printf("0x%llx ", ext);
> > + printf("0x%llx ", (unsigned long long)ext);
> >   break;
> >   }
> >   break;
> >   default:
> >   memcpy(&ext, data + i, sizeof(ext));
> > - printf("0x%llx", betoh64(ext));
> > + printf("0x%llx", (unsigned long long)be64toh(ext));
> >   }
> >   if (i + 8 < len)
> >   printf(", ");
> >   }
> >  }
> >  
> > -char *
> > -fmt_mem(int64_t num)
> > +static char *
> > +fmt_mem(long long num)
> >  {
> >   static char buf[16];
> >  
> >   if (fmt_scaled(num, buf) == -1)
> > - snprintf(buf, sizeof(buf), "%lldB", (long long)num);
> > + snprintf(buf, sizeof(buf), "%lldB", num);
> >  
> >   return (buf);
> >  }
> > @@ -1822,31 +1822,31 @@ show_rib_memory_msg(struct imsg *imsg)
> >   continue;
> >   pts += stats.pt_cnt[i] * pt_sizes[i];
> >   printf("%10lld %s network entries using %s of memory\n",
> > -    (long long)stats.pt_cnt[i], aid_vals[i].name,
> > +    stats.pt_cnt[i], aid_vals[i].name,
> >      fmt_mem(stats.pt_cnt[i] * pt_sizes[i]));
> >   }
> >   printf("%10lld rib entries using %s of memory\n",
> > -    (long long)stats.rib_cnt, fmt_mem(stats.rib_cnt *
> > +    stats.rib_cnt, fmt_mem(stats.rib_cnt *
> >      sizeof(struct rib_entry)));
> >   printf("%10lld prefix entries using %s of memory\n",
> > -    (long long)stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
> > +    stats.prefix_cnt, fmt_mem(stats.prefix_cnt *
> >      sizeof(struct prefix)));
> >   printf("%10lld BGP path attribute entries using %s of memory\n",
> > -    (long long)stats.path_cnt, fmt_mem(stats.path_cnt *
> > +    stats.path_cnt, fmt_mem(stats.path_cnt *
> >      sizeof(struct rde_aspath)));
> >   printf("\t   and holding %lld references\n",
> > -    (long long)stats.path_refs);
> > +    stats.path_refs);
> >   printf("%10lld BGP AS-PATH attribute entries using "
> >      "%s of memory\n\t   and holding %lld references\n",
> > -    (long long)stats.aspath_cnt, fmt_mem(stats.aspath_size),
> > -    (long long)stats.aspath_refs);
> > +    stats.aspath_cnt, fmt_mem(stats.aspath_size),
> > +    stats.aspath_refs);
> >   printf("%10lld BGP attributes entries using %s of memory\n",
> > -    (long long)stats.attr_cnt, fmt_mem(stats.attr_cnt *
> > +    stats.attr_cnt, fmt_mem(stats.attr_cnt *
> >      sizeof(struct attr)));
> >   printf("\t   and holding %lld references\n",
> > -    (long long)stats.attr_refs);
> > +    stats.attr_refs);
> >   printf("%10lld BGP attributes using %s of memory\n",
> > -    (long long)stats.attr_dcnt, fmt_mem(stats.attr_data));
> > +    stats.attr_dcnt, fmt_mem(stats.attr_data));
> >   printf("%10lld as-set elements in %lld tables using "
> >      "%s of memory\n", stats.aset_nmemb, stats.aset_cnt,
> >      fmt_mem(stats.aset_size));
> > Index: usr.sbin/bgpd/bgpd.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> > retrieving revision 1.372
> > diff -u -p -r1.372 bgpd.h
> > --- usr.sbin/bgpd/bgpd.h 18 Feb 2019 12:35:08 -0000 1.372
> > +++ usr.sbin/bgpd/bgpd.h 18 Feb 2019 20:53:38 -0000
> > @@ -1064,33 +1064,33 @@ extern struct rib_names ribnames;
> >  #define AS_NONE 0
> >  
> >  struct rde_memstats {
> > - int64_t path_cnt;
> > - int64_t path_refs;
> > - int64_t prefix_cnt;
> > - int64_t rib_cnt;
> > - int64_t pt_cnt[AID_MAX];
> > - int64_t nexthop_cnt;
> > - int64_t aspath_cnt;
> > - int64_t aspath_size;
> > - int64_t aspath_refs;
> > - int64_t attr_cnt;
> > - int64_t attr_refs;
> > - int64_t attr_data;
> > - int64_t attr_dcnt;
> > - int64_t aset_cnt;
> > - int64_t aset_size;
> > - int64_t aset_nmemb;
> > - int64_t pset_cnt;
> > - int64_t pset_size;
> > + long long path_cnt;
> > + long long path_refs;
> > + long long prefix_cnt;
> > + long long rib_cnt;
> > + long long pt_cnt[AID_MAX];
> > + long long nexthop_cnt;
> > + long long aspath_cnt;
> > + long long aspath_size;
> > + long long aspath_refs;
> > + long long attr_cnt;
> > + long long attr_refs;
> > + long long attr_data;
> > + long long attr_dcnt;
> > + long long aset_cnt;
> > + long long aset_size;
> > + long long aset_nmemb;
> > + long long pset_cnt;
> > + long long pset_size;
> >  };
> >  
> >  struct rde_hashstats {
> >   char name[16];
> > - int64_t num;
> > - int64_t min;
> > - int64_t max;
> > - int64_t sum;
> > - int64_t sumq;
> > + long long num;
> > + long long min;
> > + long long max;
> > + long long sum;
> > + long long sumq;
> >  };
> >  
> >  #define MRT_FILE_LEN 512
> > Index: usr.sbin/bgpd/session.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
> > retrieving revision 1.131
> > diff -u -p -r1.131 session.h
> > --- usr.sbin/bgpd/session.h 18 Feb 2019 09:58:19 -0000 1.131
> > +++ usr.sbin/bgpd/session.h 18 Feb 2019 20:53:38 -0000
> > @@ -149,22 +149,22 @@ struct ctl_conn {
> >  TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns;
> >  
> >  struct peer_stats {
> > - u_int64_t msg_rcvd_open;
> > - u_int64_t msg_rcvd_update;
> > - u_int64_t msg_rcvd_notification;
> > - u_int64_t msg_rcvd_keepalive;
> > - u_int64_t msg_rcvd_rrefresh;
> > - u_int64_t msg_sent_open;
> > - u_int64_t msg_sent_update;
> > - u_int64_t msg_sent_notification;
> > - u_int64_t msg_sent_keepalive;
> > - u_int64_t msg_sent_rrefresh;
> > - u_int64_t prefix_rcvd_update;
> > - u_int64_t prefix_rcvd_withdraw;
> > - u_int64_t prefix_rcvd_eor;
> > - u_int64_t prefix_sent_update;
> > - u_int64_t prefix_sent_withdraw;
> > - u_int64_t prefix_sent_eor;
> > + unsigned long long msg_rcvd_open;
> > + unsigned long long msg_rcvd_update;
> > + unsigned long long msg_rcvd_notification;
> > + unsigned long long msg_rcvd_keepalive;
> > + unsigned long long msg_rcvd_rrefresh;
> > + unsigned long long msg_sent_open;
> > + unsigned long long msg_sent_update;
> > + unsigned long long msg_sent_notification;
> > + unsigned long long msg_sent_keepalive;
> > + unsigned long long msg_sent_rrefresh;
> > + unsigned long long prefix_rcvd_update;
> > + unsigned long long prefix_rcvd_withdraw;
> > + unsigned long long prefix_rcvd_eor;
> > + unsigned long long prefix_sent_update;
> > + unsigned long long prefix_sent_withdraw;
> > + unsigned long long prefix_sent_eor;
> >   time_t last_updown;
> >   time_t last_read;
> >   u_int32_t prefix_cnt;
> >
> >

--
Andreas Kusalananda Kähäri,
National Bioinformatics Infrastructure Sweden (NBIS),
Uppsala University, Sweden.

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Claudio Jeker
On Tue, Feb 19, 2019 at 12:10:25AM +0100, Andreas Kusalananda Kähäri wrote:

> On Mon, Feb 18, 2019 at 10:11:03PM +0100, Mark Kettenis wrote:
> > > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > > From: Claudio Jeker <[hidden email]>
> > >
> > > In some places bgpd just wants something bigger then a 32bit int.
> > > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > > least 64bit and therefor good enough. Makes the mess with type definition
> > > of int64_t on various systems go away (including a bunch of type casts).
> > > While there also apply the endian.h cleanup done in bgpd a few days ago.
> > >
> > > OK?
> >
> > You could use <stdint.h> and uint64_t instead.  That should be
> > portable.  But you'd still need to be careful about printf statements
> > since (u)int64_t might be (unsigned) long on some systems.
>
> printf should be no issue if you use the correct PRI*64 (PRIu64 or
> PRId64) macro from <inttypes.h>.  Both <stdint.h> and <inttypes.h> are
> C99.
>
> E.g.
>
>     uint64_t thing;
>
>     /* ... */
>
>     printf("The value is %" PRIu64 "\n", thing);
>
>
> ... but I'm really not qualified to say anything about what you guys should do.
>

While true I have to say that the PRI constructs are even worse than doing
casts. Chopping up the format string like this is just ugly and unreadable
for complex format strings.

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: bgpd use long long instead of int64_t

Otto Moerbeek
On Tue, Feb 19, 2019 at 09:28:36AM +0100, Claudio Jeker wrote:

> On Tue, Feb 19, 2019 at 12:10:25AM +0100, Andreas Kusalananda Kähäri wrote:
> > On Mon, Feb 18, 2019 at 10:11:03PM +0100, Mark Kettenis wrote:
> > > > Date: Mon, 18 Feb 2019 21:59:38 +0100
> > > > From: Claudio Jeker <[hidden email]>
> > > >
> > > > In some places bgpd just wants something bigger then a 32bit int.
> > > > Instead of using int64_t or u_int64_t use (unsigned) long long which is at
> > > > least 64bit and therefor good enough. Makes the mess with type definition
> > > > of int64_t on various systems go away (including a bunch of type casts).
> > > > While there also apply the endian.h cleanup done in bgpd a few days ago.
> > > >
> > > > OK?
> > >
> > > You could use <stdint.h> and uint64_t instead.  That should be
> > > portable.  But you'd still need to be careful about printf statements
> > > since (u)int64_t might be (unsigned) long on some systems.
> >
> > printf should be no issue if you use the correct PRI*64 (PRIu64 or
> > PRId64) macro from <inttypes.h>.  Both <stdint.h> and <inttypes.h> are
> > C99.
> >
> > E.g.
> >
> >     uint64_t thing;
> >
> >     /* ... */
> >
> >     printf("The value is %" PRIu64 "\n", thing);
> >
> >
> > ... but I'm really not qualified to say anything about what you guys should do.
> >
>
> While true I have to say that the PRI constructs are even worse than doing
> casts. Chopping up the format string like this is just ugly and unreadable
> for complex format strings.

Yes, and the only other alternative I can think of: (u)intmax_t and
%jd or %ju is not very attractive either.

        -Otto