Re: [PATCH v3 7/7] tools/proctrace: add proctrace tool

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Fri, Jun 26, 2020 at 7:26 AM Ian Jackson <ian.jackson@xxxxxxxxxx> wrote:
>
> Wei Liu writes ("Re: [PATCH v3 7/7] tools/proctrace: add proctrace tool"):
> > On Mon, Jun 22, 2020 at 08:12:56PM +0200, Michał Leszczyński wrote:
> > > Add an demonstration tool that uses xc_vmtrace_* calls in order
> > > to manage external IPT monitoring for DomU.
> ...
> > > +    if (rc) {
> > > +        fprintf(stderr, "Failed to call xc_vmtrace_pt_disable\n");
> > > +        return 1;
> > > +    }
> > > +
> >
> > You should close fmem and xc in the exit path.
>
> Thanks for reviewing this.  I agree with your comments.  But I
> disagree with this one.
>
> This is in main().  When the program exits, the xc handle and memory
> mappings will go away as the kernel tears down the process.
>
> Leaving out this kind of cleanup in standalone command-line programs
> is fine, I think.  It can make the code simpler - and it avoids the
> risk of doing it wrong, which can turn errors into crashes.

Hi Ian,
while I agree that this particular code would not be an issue,
consider that these tools are often taken as sample codes to be reused
in other contexts as well. As such, I think it should include the
close bits as well and exhibit all the "best practices" we would like
to see anyone else building tools for Xen.

Cheers,
Tamas