Re: clean up size_t/ssize_t use with POSIX file system APIs - Mailing list 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