Re: sockaddr_un.sun_len vs. reality

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: sockaddr_un.sun_len vs. reality
Date: 2022-02-14 14:25:33
Message-ID: CA+Tgmob-3oNXwGaabgdntKZKKg3NCQEZdt25mO8NkhJEqWS4AQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 14, 2022 at 1:19 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-02-14 14:33:50 Re: Allow parallel plan for referential integrity checks?
Previous Message Robert Haas 2022-02-14 13:53:32 Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints