| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Cleaning up pgcrypto/crypt-des.c to avoid compiler warnings |
| Date: | 2026-09-25 18:37:30 |
| Message-ID: | CAJTYsWUs8mPCpzULs4u8O5wDiNLVvmnOUvkZtO+DttKxbN+rZQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, 25 Sept 2026 at 23:28, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> For some time now, buildfarm member serinus has been complaining
> about pgcrypto/crypt-des.c:
>
> [1601/2408] /usr/bin/ccache /usr/lib/gcc-snapshot/bin/gcc
-Icontrib/pgcrypto/pgcrypto.so.p -Isrc/include -I../pgsql/src/include
-I/usr/include/libxml2 -fdiagnostics-color=never -D_FILE_OFFSET_BITS=64
-Wall -Winvalid-pch -O2 -g -fno-strict-aliasing -fwrapv
-fexcess-precision=standard -fpch-deps -D_GNU_SOURCE -Wpointer-arith
-Werror=vla -Wmissing-format-attribute -Wcast-function-type
-Wshadow=compatible-local -Wformat-security -Wmissing-prototypes
-Wold-style-declaration -Wold-style-definition -Wstrict-prototypes
-Wimplicit-fallthrough=5 -Wdeclaration-after-statement
-Wmissing-variable-declarations -Wno-format-truncation
-Wno-stringop-truncation -O1 -ggdb -g3 -fno-omit-frame-pointer -Wall
-Wextra -Wno-unused-parameter -Wno-sign-compare
-Wno-missing-field-initializers -O3 -Wno-unterminated-string-initialization
-Wno-clobbered -fPIC -isystem /usr/include/mit-krb5 -isystem
/usr/include/mit-krb5 -pthread -fvisibility=hidden -MD -MQ
contrib/pgcrypto/pgcrypto.so.p/crypt-des.c.o -MF
contrib/pgcrypto/pgcrypto.so.p/crypt-des.c.o.d -o
contrib/pgcrypto/pgcrypto.so.p/crypt-des.c.o -c
../pgsql/contrib/pgcrypto/crypt-des.c
> ../pgsql/contrib/pgcrypto/crypt-des.c: In function 'px_crypt_des':
> ../pgsql/contrib/pgcrypto/crypt-des.c:675:22: warning: writing 8 bytes
into a region of size 7 [-Wstringop-overflow=]
> 675 | *q++ = *key << 1;
> | ~~~~~^~~~~~~~~~~
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [1, 2] into
destination object 'keybuf' of size 8
> 659 | keybuf[2];
> | ^~~~~~
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [2, 3] into
destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [3, 4] into
destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [4, 5] into
destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [5, 6] into
destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [6, 7] into
destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [7, 8] into
destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:675:22: warning: writing 1 byte
into a region of size 0 [-Wstringop-overflow=]
> 675 | *q++ = *key << 1;
> | ~~~~~^~~~~~~~~~~
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset 8 into
destination object 'keybuf' of size 8
> 659 | keybuf[2];
> | ^~~~~~
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [9, 10]
into destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [10, 11]
into destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [11, 12]
into destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [12, 13]
into destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [13, 14]
into destination object 'keybuf' of size 8
> ../pgsql/contrib/pgcrypto/crypt-des.c:659:33: note: at offset [14, 15]
into destination object 'keybuf' of size 8
>
> It's possible to see by inspection that this loop doesn't actually
> overrun the buffer. But the termination condition is oddly written:
>
> q = (uint8 *) keybuf;
> while (q - (uint8 *) keybuf - 8)
>
> I think this odd coding, perhaps combined with all the cowboy casting
> that's going on here, is what's confusing gcc into giving a warning.
> I propose the attached patch to make this code less ugly and (with
> luck) suppress the warning.
Yeah I agree that it's pretty ugly.
We had an open patch for that obscure loop earlier[1].
(I'll close that commitfest item)
I reviewed your patch and it looks good and clearer to me.
Thanks for the patch.
Regards,
Ayush
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-09-25 18:54:45 | Re: [PATCH] btree_gist: add cross-type integer operator support for GiST |
| Previous Message | Dirkjan Bussink | 2026-09-25 18:27:56 | Re: Server crash when describing a FETCH statement after its cursor is closed |