Re: Feature request: ZSH_XTRACEFD variable

["Timothée Mazzucotelli" <timothee.mazzucotelli@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

Thank you very much Daniel for this great explanation
on what leaking a FILE (or a fd) means. It's much clearer now.

From what I understand, the code for the ZSH_XTRACEFD feature
will then not leak any file descriptor itself.

Based on the man pages:

> The *fdopen*() function associates a stream with the existing file
descriptor, *fd*. [...]
> The file descriptor is not dup'ed, and will be closed when the stream
created by *fdopen*() is closed.

The user is the one creating the file descriptor initially,
and all Zsh does is assigning it to the xtrerr variable.

> xtrerr = fdopen(fd, "w");

Zsh should not close the file descriptor itself.
As stated previously, it should be the user's choice and responsibility
to close the file descriptors they have opened.

If, however, the stream associated to the fd by fdopen allocates memory,
then not closing that stream and assigning another value to xtrerr would
indeed cause a memory leak.
In that case, and now that I understand Bart's answer under the light of
your explanation,
we could fdopen a duplicate of the fd instead,
so we can close both the duplicated fd and its associated stream, fixing
the memory leak.

We could use the valgrind test suite to make sure there's a memory leak or
not,
unfortunately I did not manage to have it working on my computer
(there were a lot of reported leaks when there should not have been any,
see previous messages).

Timothée

On Fri, Sep 4, 2020 at 9:54 PM Bart Schaefer <schaefer@brasslantern.com>
wrote:

> On Fri, Sep 4, 2020 at 12:49 PM Daniel Shahaf <d.s@daniel.shahaf.name>
> wrote:
> >
> > zsh: cannot moved fd 3: too many open files
> >
> > P.S.  The quoted error message is ungrammatical.
>
> That is supposed to say "cannot movefd fd 3" ... movefd being the name
> of the function ... someone has done a spell-checking pass over the
> error messages and fixed a "mistake" that wasn't actually a mistake?
>
>

[-- Attachment #2: Type: text/html, Size: 3129 bytes --]