Thanks, Padraig — understood.
While the current implementation works correctly, it might still be a good
idea to
explicitly close the file once it’s no longer in use.

thanks,
CheckScope

On Mon, Nov 10, 2025 at 7:26 PM Pádraig Brady <[email protected]> wrote:

> tag 79808 notabug
> close 79808
> stop
>
> details below...
>
> On 10/11/2025 09:45, Ray steven wrote:
> > Hello coreutils maintainers,
> >
> > While reviewing the source code of GNU coreutils (tac.c), I noticed a
> small
> > resource management
> > issue in the function `tac_nonseekable()`:
> >
> > After calling `copy_to_temp(&tmp_stream, &tmp_file, input_fd, file)`, the
> > function uses `tmp_stream`
> > in `tac_seekable(fileno(tmp_stream), tmp_file, bytes_copied)` but never
> > calls `fclose(tmp_stream)` afterwards.
> > As a result, the FILE stream and its underlying file descriptor remain
> open
> > until process termination.
> >
> > Although this does not cause user-visible problems for short-lived `tac`
> > executions, adding an explicit
> > `fclose(tmp_stream)` (and perhaps removing the temporary file) would
> > improve resource hygiene and
> > avoid potential descriptor exhaustion if the function were reused in a
> > long-running context.
> >
> > Environment:
> > - Observed in current `src/tac.c` (line ~423 in GNU coreutils latest)
> >
> > Suggested fix (simplified):
> > ```c
> > bool ok = tac_seekable(fileno(tmp_stream), tmp_file, bytes_copied);
> > fclose(tmp_stream);
> > unlink(tmp_file);
> > return ok;
>
> The temp-stream module manages a single global resource per process,
> so this would be incorrect.
> I verified that there is no leak with `tac - - - - -`.
>
> thanks,
> Padraig
>

Reply via email to