Re: sockaddr_un.sun_len vs. reality - Mailing list pgsql-hackers

From Robert Haas
Subject Re: sockaddr_un.sun_len vs. reality
Date
Msg-id CA+Tgmob-3oNXwGaabgdntKZKKg3NCQEZdt25mO8NkhJEqWS4AQ@mail.gmail.com
Whole thread Raw
In response to sockaddr_un.sun_len vs. reality  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: sockaddr_un.sun_len vs. reality  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: sockaddr_un.sun_len vs. reality  (Thomas Munro <thomas.munro@gmail.com>)
List pgsql-hackers
On Mon, Feb 14, 2022 at 1:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> For a very long time, the AIX 7.1 buildfarm members (sungazer and tern)
> have complained about
>
> ip.c:236:17: warning: conversion from 'long unsigned int' to 'uchar_t' {aka 'unsigned char'} changes value from
'1025'to '1' [-Woverflow]
 
>
> I'd ignored this as not being very interesting, but I got motivated to
> recheck it today.  The warning is coming from
>
> #ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
>         unp->sun_len = sizeof(struct sockaddr_un);
> #endif
>
> so we can infer that the sun_len field is unsigned char and the
> size of struct sockaddr_un doesn't fit.
>
> Poking around a bit, I find that sun_len exists on most BSD-ish
> platforms and it seems to be unsigned char (almost?) everywhere.
> But on other platforms sizeof(struct sockaddr_un) is only a bit
> over 100, so there's not an overflow problem.
>
> It's not real clear that there's any problem on AIX either;
> given that the regression tests pass, I strongly suspect that
> nothing is paying attention to the sun_len field.  But if we're
> going to fill it in at all, we should probably try to fill it
> in with something less misleading than "1".

It's not real clear to me that it's worth complicating the code to
avoid a harmless compiler warning on an 11-year-old operating system
with minimal real-world usage. On the other hand, if you really feel
motivated to do something about it, I'm not here to argue. My one
suggestion is that if you do add some strange incantation here along
the lines you propose, you should at least add a comment explaining
that it was done to suppress a warning on AIX 7.1. That way, there's a
chance someone might be brave enough to try removing it in the future
at such time as nobody cares about AIX 7.1 any more.

-- 
Robert Haas
EDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints
Next
From: Robert Haas
Date:
Subject: Re: Allow parallel plan for referential integrity checks?