Re: clean up size_t/ssize_t use with POSIX file system APIs

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: clean up size_t/ssize_t use with POSIX file system APIs
Date: 2026-07-06 12:38:48
Message-ID: 6c2e3346-62ed-4d2c-b41c-44491f0bddb2@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 29/06/2026 15:04, Peter Eisentraut wrote:
> Here is a patch series to clean up the use of size_t and ssize_t with
> POSIX file system APIs (read, write, etc.) and the APIs that PostgreSQL
> has built on top.
>
> Most newer code already did this correctly, but some older code did not
> and mostly used int or some other randomness.  Which also sometimes
> leads to odd back-and-forth at different API layers.
>
> Aside from consistency and basic correctness, this also helps in other
> ways.
>
> It makes error messages more consistent.  Consider the current potpourri
> like
>
> read %d of %d
> read %d of %lld
> read %d of %u
> read %d of %zu
> read %lld of %lld
> read %zd of %lld
> read %zd of %zu
> read %zu of %zu
>
> Once we get all of this aligned better, there might also be some
> opportunities to refactor some repeated combinations like read/write +
> error checking + pgstat report.
>
> Another advantage of correctly using size_t instead of int is that you
> don't have to wonder whether a negative value might be passed and what
> to do with it.
>
> Also, of course, we do want to actually support large files in some
> situations.  I don't think it currently matters in practice for the
> places I changed here, but it might in the future, or if this code gets
> expanded or copied around.
>
> Some patches have some additional details that are noted in the commit
> messages, but most of it is relatively straightforward replacement and
> some cosmetic adjustments.
>
> (Note: There are similar issues with network functions recv()/send(),
> but there are weird complications related to Windows, different API
> wrappers in used for frontends and backends, so I left those out for now.)
>
> The patches related to casts of pgoff_t are included here (0002 and
> 0003) here because they are prerequisites, but there is a separate
> thread for those at [0].
>
> [0]: https://www.postgresql.org/message-id/20ce62fa-47fc-457b-
> b504-12f3c1651726%40eisentraut.org

I skimmed through all these patches (phew!), and it all looks good to
me. Only this one little thing caught my eye:

> diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
> index b5433a75d12..85ded4c0e2b 100644
> --- a/src/bin/pg_ctl/pg_ctl.c
> +++ b/src/bin/pg_ctl/pg_ctl.c
> @@ -317,11 +317,12 @@ readfile(const char *path, int *numlines)
> int fd;
> int nlines;
> char **result;
> + size_t buflen;
> char *buffer;
> char *linebegin;
> int i;
> int n;
> - int len;
> + ssize_t nread;
> struct stat statbuf;
>
> *numlines = 0; /* in case of failure or empty file */
> @@ -367,7 +370,7 @@ readfile(const char *path, int *numlines)
> * any characters after the last newline will be ignored.
> */
> nlines = 0;
> - for (i = 0; i < len; i++)
> + for (i = 0; i < nread; i++)
> {
> if (buffer[i] == '\n')
> nlines++;
> @@ -380,7 +383,7 @@ readfile(const char *path, int *numlines)
> /* now split the buffer into lines */
> linebegin = buffer;
> n = 0;
> - for (i = 0; i < len; i++)
> + for (i = 0; i < nread; i++)
> {
> if (buffer[i] == '\n')
> {

Should probably change the 'i' into size_t as well, since it's compared
with 'nread'. And 'nlines' too, to be pendantic, so that it doesn't
overflow if you have a 10 GB file full of newlines..

As a sanity check though, it'd probably make sense to check that the
file is not larger than a few kB or so. This is used to read the pid
file and postmaster.opts file, neither of which is expected to be large.

- Heikki

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jakub Wartak 2026-07-06 12:42:10 Re: hang during shutdown
Previous Message Thomas Munro 2026-07-06 12:32:12 Re: pg_threads.h take II