Am 20.12.24 um 22:16 schrieb Simon Kelley:
On 12/20/24 12:27, Matthias Andree via Dnsmasq-discuss wrote:
Simon,
I cannot compile v2.91test1 on FreeBSD 14.2, errors below. (Neither
tarball nor Git compile.) (2nd to last shown errors). Patch attached,
should be fine with git-am.
Patch applied, thanks.
Adding multiple unions with variable size into some other aggregate
(union or struct) isn't portable, first shown warning below, might be
an error on other or strictly set compilers. This needs fixing not
covered by my patch.
If I've understood this right, then
https://thekelleys.org.uk/gitweb/?
p=dnsmasq.git;a=commit;h=4902807879c9b39db46c32e3e082a33266836d24
should fix it.
Thanks, but it's not sufficient, and my wording was unclear, sorry for
that.
I am attaching an incremental git-am ready patch to go on top your Git HEAD,
to fix all sorts of issues and make this conforming C99 with default
options set,
and fix another load of warnings you receive when setting the compiler
to pick the nits,
-pedantic-errors -std=c99 (or c11, c18, c2x).
It changes many void * to uint8_t * to make the "increment by bytes"
explicit.
You can't do:
void *foo;
// ...
foo += 2.
IMPORTANT: please review the char data[1] in my patch, whether you want
to increase the size.
I see the six header warnings show below for practically all source
files compiled, and regarding the log message:
The warning is from FreeBSD 14.2's clang 18.1.6 compiler, not GCC, which
only shows them under -pedantic (I used GCC 13.3):
The root cause for these warnings appears to be the VLA char data[]
inside union all_addr's struct datablock, see line 342 of src/dnsmasq.h:
336 /* for arbitrary RR record small enough to go in addr.
337 NOTE: rrblock and rrdata are discriminated by the
F_KEYTAG bit
338 in the cache flags. */
339 struct datablock {
340 unsigned short rrtype;
341 unsigned char datalen; /* also length of SOA in negative
records. */
342 char data[];
343 } rrdata;
344 };
Declaring this as char data[1] or data[16] or whatever is suitable
should fix the warnings. data[0] is non-conforming and won't work either.
I don't know what's a good size here, as I haven't read enough context
code to judge that. My patch sets it 1 because that's the minimum in
order not to bloat the structure.
Also note that many of the *_addr types you define are
memory-inefficient because you often have short types before pointer
types, which have alignment requirements almost everywhere, and
something like:
struct {
unsigned short rrtype;
unsigned short datalen;
struct blockdata *rrdata;
} rrblock;
will incur 4 bytes of padding between datalen and rrdata so the pointer
is aligned at 64-bit boundaries. The standing recommendation is to sort
members by descending order of alignment requirement and size. I. e.
pointers and longs first, then ints, then shorts.
For this particular example, you could write:
struct {
struct blockdata *rrdata;
unsigned short rrtype;
unsigned short datalen;
} rrblock;
to fix that. IMPORTANT: my patch does not address this!
In many cases, size-definite types (such as uint16_t) are a good choice
if you really know the value range, say, from IETF standards and RFCs,
but would require lots of printf-style format adjustments, too.
./dnsmasq.h:350:18: warning: field 'addr' with variable sized type
'union all_addr' not at the end of a struct or class is a GNU
extension [-Wgnu-variable-sized-type-not-at-end]
350 | union all_addr addr;
| ^
./dnsmasq.h:416:18: warning: field 'addr' with variable sized type
'union all_addr' not at the end of a struct or class is a GNU
extension [-Wgnu-variable-sized-type-not-at-end]
416 | union all_addr addr;
| ^
./dnsmasq.h:483:18: warning: field 'addr' with variable sized type
'union all_addr' not at the end of a struct or class is a GNU
extension [-Wgnu-variable-sized-type-not-at-end]
483 | union all_addr addr;
| ^
./dnsmasq.h:782:20: warning: field 'dest' with variable sized type
'union all_addr' not at the end of a struct or class is a GNU
extension [-Wgnu-variable-sized-type-not-at-end]
782 | union all_addr dest;
| ^
./dnsmasq.h:787:5: warning: field 'frec_src' with variable sized type
'struct frec_src' not at the end of a struct or class is a GNU
extension [-Wgnu-variable-sized-type-not-at-end]
787 | } frec_src;
| ^
./dnsmasq.h:1108:18: warning: field 'source' with variable sized type
'union all_addr' not at the end of a struct or class is a GNU
extension [-Wgnu-variable-sized-type-not-at-end]
1108 | union all_addr source;
| ^
GCC shows such warnings in stricter modes (for instance, -pedantic),
clang already does with default options.
There is also this char * vs. void * complaint:
bpf.c: In function 'arp_enumerate':
bpf.c:94:40: warning: passing argument 2 of 'callback.af_unspec' from
incompatible pointer type [-Wincompatible-pointer-types]
94 | if (!callback.af_unspec(AF_INET, &sin2->sin_addr,
LLADDR(sdl), sdl->sdl_alen, parm))
| ^~~~~~~~~~~~~~~
| |
| struct in_addr *
bpf.c:94:40: note: expected 'char *' but argument is of type 'struct
in_addr *'
A simple fix for this one is changing the inf (*af_unspec)...
declaration to make void *addrp instead of char *addrp.
From de0bb5e5b793ef843f69f92e84e3ea710c4f23c3 Mon Sep 17 00:00:00 2001
From: Matthias Andree <matthias.and...@gmx.de>
Date: Sun, 22 Dec 2024 11:34:48 +0100
Subject: [PATCH] Make this compile under -pedantic-errors -std=c99, c11, c18
We specifically avoid missing prototypes,
pointer arithmetic on void * pointers,
empty aggregate initializer braces,
type mismatch on af_unspec's 2nd argument,
variable-length array in all_addr.rrdata's struct datablock.
On FreeBSD 14.2 amd64, the code now compiles with various GCC and clang
with -std=cXX and -pedantic-errors options.
Signed-off-by: Matthias Andree <matthias.and...@gmx.de>
---
src/arp.c | 2 +-
src/blockdata.c | 2 +-
src/dnsmasq.h | 4 ++--
src/edns0.c | 2 +-
src/forward.c | 2 +-
src/loop.c | 2 +-
src/network.c | 2 +-
src/outpacket.c | 12 ++++++------
src/rfc3315.c | 24 ++++++++++++------------
src/util.c | 2 +-
10 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/arp.c b/src/arp.c
index 6ff1f01..8c21655 100644
--- a/src/arp.c
+++ b/src/arp.c
@@ -35,7 +35,7 @@ struct arp_record {
static struct arp_record *arps = NULL, *old = NULL, *freelist = NULL;
static time_t last = 0;
-static int filter_mac(int family, char *addrp, char *mac, size_t maclen, void *parmv)
+static int filter_mac(int family, void *addrp, char *mac, size_t maclen, void *parmv)
{
struct arp_record *arp;
diff --git a/src/blockdata.c b/src/blockdata.c
index 9768369..1990a8c 100644
--- a/src/blockdata.c
+++ b/src/blockdata.c
@@ -193,7 +193,7 @@ void *blockdata_retrieve(struct blockdata *block, size_t len, void *data)
{
size_t blen;
struct blockdata *b;
- void *new, *d;
+ uint8_t *new, *d;
static unsigned int buff_len = 0;
static unsigned char *buff = NULL;
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index a9ac815..3289e74 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -339,7 +339,7 @@ union all_addr {
struct datablock {
unsigned short rrtype;
unsigned char datalen; /* also length of SOA in negative records. */
- char data[];
+ char data[1];
} rrdata;
};
@@ -1667,7 +1667,7 @@ void route_sock(void);
/* bpf.c or netlink.c */
typedef union {
- int (*af_unspec)(int family, char *addrp, char *mac, size_t maclen, void *parmv);
+ int (*af_unspec)(int family, void *addrp, char *mac, size_t maclen, void *parmv);
int (*af_inet)(struct in_addr local, int if_index, char *label, struct in_addr netmask, struct in_addr broadcast, void *vparam);
int (*af_inet6)(struct in6_addr *local, int prefix, int scope, int if_index, int flags, unsigned int preferred, unsigned int valid, void *vparam);
int (*af_local)(int index, unsigned int type, char *mac, size_t maclen, void *parm);
diff --git a/src/edns0.c b/src/edns0.c
index fc19f31..1477a39 100644
--- a/src/edns0.c
+++ b/src/edns0.c
@@ -491,7 +491,7 @@ static size_t add_umbrella_opt(struct dns_header *header, size_t plen, unsigned
{
*cacheable = 0;
- struct umbrella_opt opt = {{"ODNS"}, UMBRELLA_VERSION, 0, {}};
+ struct umbrella_opt opt = {{"ODNS"}, UMBRELLA_VERSION, 0, {0}};
u8 *u = &opt.fields[0];
int family = source->sa.sa_family;
int size = family == AF_INET ? INADDRSZ : IN6ADDRSZ;
diff --git a/src/forward.c b/src/forward.c
index fa696e5..4184ca8 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -3054,7 +3054,7 @@ static struct frec *lookup_frec(char *target, int class, int rrtype, int id, int
}
/* Send query packet again, if we can. */
-void resend_query()
+void resend_query(void)
{
if (daemon->srv_save)
server_send(daemon->srv_save, daemon->fd_save,
diff --git a/src/loop.c b/src/loop.c
index f87293f..8022589 100644
--- a/src/loop.c
+++ b/src/loop.c
@@ -19,7 +19,7 @@
#ifdef HAVE_LOOP
static ssize_t loop_make_probe(u32 uid);
-void loop_send_probes()
+void loop_send_probes(void)
{
struct server *serv;
struct randfd_list *rfds = NULL;
diff --git a/src/network.c b/src/network.c
index 36d9262..f66fe31 100644
--- a/src/network.c
+++ b/src/network.c
@@ -647,7 +647,7 @@ static int iface_allowed_v4(struct in_addr local, int if_index, char *label,
/*
* Clean old interfaces no longer found.
*/
-static void clean_interfaces()
+static void clean_interfaces(void)
{
struct irec *iface;
struct irec **up = &daemon->interfaces;
diff --git a/src/outpacket.c b/src/outpacket.c
index 1a29f61..ebaf6b5 100644
--- a/src/outpacket.c
+++ b/src/outpacket.c
@@ -23,7 +23,7 @@ static size_t outpacket_counter;
void end_opt6(int container)
{
- void *p = daemon->outpacket.iov_base + container + 2;
+ uint8_t *p = (uint8_t *)daemon->outpacket.iov_base + container + 2;
u16 len = outpacket_counter - container - 4 ;
PUTSHORT(len, p);
@@ -50,11 +50,11 @@ int save_counter(int newval)
void *expand(size_t headroom)
{
- void *ret;
+ uint8_t *ret;
if (expand_buf(&daemon->outpacket, outpacket_counter + headroom))
{
- ret = daemon->outpacket.iov_base + outpacket_counter;
+ ret = (uint8_t *)daemon->outpacket.iov_base + outpacket_counter;
outpacket_counter += headroom;
return ret;
}
@@ -65,7 +65,7 @@ void *expand(size_t headroom)
int new_opt6(int opt)
{
int ret = outpacket_counter;
- void *p;
+ unsigned char *p;
if ((p = expand(4)))
{
@@ -88,7 +88,7 @@ void *put_opt6(void *data, size_t len)
void put_opt6_long(unsigned int val)
{
- void *p;
+ unsigned char *p;
if ((p = expand(4)))
PUTLONG(val, p);
@@ -96,7 +96,7 @@ void put_opt6_long(unsigned int val)
void put_opt6_short(unsigned int val)
{
- void *p;
+ uint8_t *p;
if ((p = expand(2)))
PUTSHORT(val, p);
diff --git a/src/rfc3315.c b/src/rfc3315.c
index ccf513f..3f6401f 100644
--- a/src/rfc3315.c
+++ b/src/rfc3315.c
@@ -39,8 +39,8 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu
static void log6_opts(int nest, unsigned int xid, void *start_opts, void *end_opts);
static void log6_packet(struct state *state, char *type, struct in6_addr *addr, char *string);
static void log6_quiet(struct state *state, char *type, struct in6_addr *addr, char *string);
-static void *opt6_find (void *opts, void *end, unsigned int search, unsigned int minsize);
-static void *opt6_next(void *opts, void *end);
+static void *opt6_find (uint8_t *opts, uint8_t *end, unsigned int search, unsigned int minsize);
+static void *opt6_next(uint8_t *opts, uint8_t *end);
static unsigned int opt6_uint(unsigned char *opt, int offset, int size);
static void get_context_tag(struct state *state, struct dhcp_context *context);
static int check_ia(struct state *state, void *opt, void **endp, void **ia_option);
@@ -61,11 +61,11 @@ static void calculate_times(struct dhcp_context *context, unsigned int *min_time
#define opt6_len(opt) ((int)(opt6_uint(opt, -2, 2)))
#define opt6_type(opt) (opt6_uint(opt, -4, 2))
-#define opt6_ptr(opt, i) ((void *)&(((unsigned char *)(opt))[4+(i)]))
+#define opt6_ptr(opt, i) ((void *)&(((uint8_t *)(opt))[4+(i)]))
-#define opt6_user_vendor_ptr(opt, i) ((void *)&(((unsigned char *)(opt))[2+(i)]))
+#define opt6_user_vendor_ptr(opt, i) ((void *)&(((uint8_t *)(opt))[2+(i)]))
#define opt6_user_vendor_len(opt) ((int)(opt6_uint(opt, -4, 2)))
-#define opt6_user_vendor_next(opt, end) (opt6_next(((void *) opt) - 2, end))
+#define opt6_user_vendor_next(opt, end) (opt6_next(((uint8_t *) opt) - 2, end))
unsigned short dhcp6_reply(struct dhcp_context *context, int interface, char *iface_name,
@@ -107,11 +107,11 @@ unsigned short dhcp6_reply(struct dhcp_context *context, int interface, char *if
static int dhcp6_maybe_relay(struct state *state, unsigned char *inbuff, size_t sz,
struct in6_addr *client_addr, int is_unicast, time_t now)
{
- void *end = inbuff + sz;
- void *opts = inbuff + 34;
+ uint8_t *end = inbuff + sz;
+ uint8_t *opts = inbuff + 34;
int msg_type = *inbuff;
unsigned char *outmsgtypep;
- void *opt;
+ uint8_t *opt;
struct dhcp_vendor *vendor;
/* if not an encapsulated relayed message, just do the stuff */
@@ -232,7 +232,7 @@ static int dhcp6_maybe_relay(struct state *state, unsigned char *inbuff, size_t
for (opt = opts; opt; opt = opt6_next(opt, end))
{
- if (opt6_ptr(opt, 0) + opt6_len(opt) > end)
+ if ((uint8_t *)opt6_ptr(opt, 0) + opt6_len(opt) > end)
return 0;
/* Don't copy MAC address into reply. */
@@ -1307,7 +1307,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu
reallocated. */
((unsigned char *)(daemon->outpacket.iov_base))[start_msg] = outmsgtype;
- log6_opts(0, state->xid, daemon->outpacket.iov_base + start_opts, daemon->outpacket.iov_base + save_counter(-1));
+ log6_opts(0, state->xid, (uint8_t *)daemon->outpacket.iov_base + start_opts, (uint8_t *)daemon->outpacket.iov_base + save_counter(-1));
return 1;
@@ -2093,7 +2093,7 @@ static void log6_packet(struct state *state, char *type, struct in6_addr *addr,
string ? string : "");
}
-static void *opt6_find (void *opts, void *end, unsigned int search, unsigned int minsize)
+static void *opt6_find (uint8_t *opts, uint8_t *end, unsigned int search, unsigned int minsize)
{
u16 opt, opt_len;
void *start;
@@ -2120,7 +2120,7 @@ static void *opt6_find (void *opts, void *end, unsigned int search, unsigned int
}
}
-static void *opt6_next(void *opts, void *end)
+static void *opt6_next(uint8_t *opts, uint8_t *end)
{
u16 opt_len;
diff --git a/src/util.c b/src/util.c
index 933340c..76cfdf5 100644
--- a/src/util.c
+++ b/src/util.c
@@ -41,7 +41,7 @@ static u32 in[12];
static u32 out[8];
static int outleft = 0;
-void rand_init()
+void rand_init(void)
{
int fd = open(RANDFILE, O_RDONLY);
--
2.47.1
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
