ntpd: prevent duplicate definitions of `conf` and `ibus_dns`

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

ntpd: prevent duplicate definitions of `conf` and `ibus_dns`

Michael Forney
This prevents a linking error with gcc 10, which enables -fno-common
by default.

ISO C requires exactly one definition of objects with external
linkage throughout the entire program.

`conf` is already defined in ntpd.c and declared extern in ntpd.h,
so the definition in parse.y is redundant.

The two definitions of `ibuf_dns` are distinct and local to their
respective files, so make them static.
---
It looks like while the ibuf_dns variables are distinct, only one
or the other is used (one through ntp_main() and the other through
ntp_dns()). So, an alternative is to add an extern declaration in
ntpd.h.

I noticed that there are quite a few places where static could be
used, but isn't, so I'm not sure which approach is preferred.

 usr.sbin/ntpd/ntp.c     | 2 +-
 usr.sbin/ntpd/ntp_dns.c | 2 +-
 usr.sbin/ntpd/parse.y   | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/usr.sbin/ntpd/ntp.c b/usr.sbin/ntpd/ntp.c
index ea9a4e92274..9cb74f1d8da 100644
--- a/usr.sbin/ntpd/ntp.c
+++ b/usr.sbin/ntpd/ntp.c
@@ -42,7 +42,7 @@
 
 volatile sig_atomic_t ntp_quit = 0;
 struct imsgbuf *ibuf_main;
-struct imsgbuf *ibuf_dns;
+static struct imsgbuf *ibuf_dns;
 struct ntpd_conf *conf;
 struct ctl_conns ctl_conns;
 u_int peer_cnt;
diff --git a/usr.sbin/ntpd/ntp_dns.c b/usr.sbin/ntpd/ntp_dns.c
index 2e1a978338a..2dbd79dada6 100644
--- a/usr.sbin/ntpd/ntp_dns.c
+++ b/usr.sbin/ntpd/ntp_dns.c
@@ -39,7 +39,7 @@
 #include "ntpd.h"
 
 volatile sig_atomic_t quit_dns = 0;
-struct imsgbuf *ibuf_dns;
+static struct imsgbuf *ibuf_dns;
 
 void sighdlr_dns(int);
 int dns_dispatch_imsg(struct ntpd_conf *);
diff --git a/usr.sbin/ntpd/parse.y b/usr.sbin/ntpd/parse.y
index 8d7ab09de34..533f67f1b8f 100644
--- a/usr.sbin/ntpd/parse.y
+++ b/usr.sbin/ntpd/parse.y
@@ -57,7 +57,6 @@ int lgetc(int);
 int lungetc(int);
 int findeol(void);
 
-struct ntpd_conf *conf;
 struct sockaddr_in query_addr4;
 struct sockaddr_in6 query_addr6;
 int poolseqnum;
--
2.26.0

Reply | Threaded
Open this post in threaded view
|

Re: ntpd: prevent duplicate definitions of `conf` and `ibus_dns`

Otto Moerbeek
On Sat, Apr 04, 2020 at 04:00:50PM -0700, Michael Forney wrote:

> This prevents a linking error with gcc 10, which enables -fno-common
> by default.
>
> ISO C requires exactly one definition of objects with external
> linkage throughout the entire program.
>
> `conf` is already defined in ntpd.c and declared extern in ntpd.h,
> so the definition in parse.y is redundant.
>
> The two definitions of `ibuf_dns` are distinct and local to their
> respective files, so make them static.
> ---
> It looks like while the ibuf_dns variables are distinct, only one
> or the other is used (one through ntp_main() and the other through
> ntp_dns()). So, an alternative is to add an extern declaration in
> ntpd.h.
>
> I noticed that there are quite a few places where static could be
> used, but isn't, so I'm not sure which approach is preferred.

Thanks, committed. Opinions differ on static for non-library symbols.

        -Otto

>
>  usr.sbin/ntpd/ntp.c     | 2 +-
>  usr.sbin/ntpd/ntp_dns.c | 2 +-
>  usr.sbin/ntpd/parse.y   | 1 -
>  3 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/usr.sbin/ntpd/ntp.c b/usr.sbin/ntpd/ntp.c
> index ea9a4e92274..9cb74f1d8da 100644
> --- a/usr.sbin/ntpd/ntp.c
> +++ b/usr.sbin/ntpd/ntp.c
> @@ -42,7 +42,7 @@
>  
>  volatile sig_atomic_t ntp_quit = 0;
>  struct imsgbuf *ibuf_main;
> -struct imsgbuf *ibuf_dns;
> +static struct imsgbuf *ibuf_dns;
>  struct ntpd_conf *conf;
>  struct ctl_conns ctl_conns;
>  u_int peer_cnt;
> diff --git a/usr.sbin/ntpd/ntp_dns.c b/usr.sbin/ntpd/ntp_dns.c
> index 2e1a978338a..2dbd79dada6 100644
> --- a/usr.sbin/ntpd/ntp_dns.c
> +++ b/usr.sbin/ntpd/ntp_dns.c
> @@ -39,7 +39,7 @@
>  #include "ntpd.h"
>  
>  volatile sig_atomic_t quit_dns = 0;
> -struct imsgbuf *ibuf_dns;
> +static struct imsgbuf *ibuf_dns;
>  
>  void sighdlr_dns(int);
>  int dns_dispatch_imsg(struct ntpd_conf *);
> diff --git a/usr.sbin/ntpd/parse.y b/usr.sbin/ntpd/parse.y
> index 8d7ab09de34..533f67f1b8f 100644
> --- a/usr.sbin/ntpd/parse.y
> +++ b/usr.sbin/ntpd/parse.y
> @@ -57,7 +57,6 @@ int lgetc(int);
>  int lungetc(int);
>  int findeol(void);
>  
> -struct ntpd_conf *conf;
>  struct sockaddr_in query_addr4;
>  struct sockaddr_in6 query_addr6;
>  int poolseqnum;
> --
> 2.26.0
>