This series addresses some Coverity reports.  To handle closing FDs, a
state struct is introduced to track FDs closed in both main() and
data_loop().

Jason Andryuk (8):
  vchan-socket-proxy: Ensure UNIX path NUL terminated
  vchan-socket-proxy: Check xs_watch return value
  vchan-socket-proxy: Unify main return value
  vchan-socket-proxy: Use a struct to store state
  vchan-socket-proxy: Switch data_loop() to take state
  vchan-socket-proxy: Set closed FDs to -1
  vchan-socket-proxy: Cleanup resources on exit
  vchan-socket-proxy: Handle closing shared input/output_fd

 tools/libvchan/vchan-socket-proxy.c | 164 ++++++++++++++++++----------
 1 file changed, 106 insertions(+), 58 deletions(-)

-- 
2.25.1

On Sun, May 24, 2020 at 10:50 PM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> This series addresses some Coverity reports.  To handle closing FDs, a
> state struct is introduced to track FDs closed in both main() and
> data_loop().

I've realized the changes here are insufficient to handle the FD
leaks.  That is, the accept()-ed FDs need to be closed inside the for
loop so they aren't leaked with each iteration.  I'll re-work for a
v2.

-Jason

On Mon, May 25, 2020 at 6:36 PM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> On Sun, May 24, 2020 at 10:50 PM Jason Andryuk <jandryuk@gmail.com> wrote:
> >
> > This series addresses some Coverity reports.  To handle closing FDs, a
> > state struct is introduced to track FDs closed in both main() and
> > data_loop().
>
> I've realized the changes here are insufficient to handle the FD
> leaks.  That is, the accept()-ed FDs need to be closed inside the for
> loop so they aren't leaked with each iteration.  I'll re-work for a
> v2.

So it turns out this series doesn't leak FDs in the for loop.  FDs are
necessarily closed down in data_loop() when the read() returns 0.  The
only returns from data_loop() are after the FDs have been closed.
data_loop() and some of the functions it calls will call exit(1) on
error, but that won't leak FDs.

Please review this series.  Sorry for the confusion.

Regards,
Jason

On Wed, May 27, 2020 at 10:59:55PM -0400, Jason Andryuk wrote:
> On Mon, May 25, 2020 at 6:36 PM Jason Andryuk <jandryuk@gmail.com> wrote:
> >
> > On Sun, May 24, 2020 at 10:50 PM Jason Andryuk <jandryuk@gmail.com> wrote:
> > >
> > > This series addresses some Coverity reports.  To handle closing FDs, a
> > > state struct is introduced to track FDs closed in both main() and
> > > data_loop().
> >
> > I've realized the changes here are insufficient to handle the FD
> > leaks.  That is, the accept()-ed FDs need to be closed inside the for
> > loop so they aren't leaked with each iteration.  I'll re-work for a
> > v2.
> 
> So it turns out this series doesn't leak FDs in the for loop.  FDs are
> necessarily closed down in data_loop() when the read() returns 0.  The
> only returns from data_loop() are after the FDs have been closed.
> data_loop() and some of the functions it calls will call exit(1) on
> error, but that won't leak FDs.
> 
> Please review this series.  Sorry for the confusion.

For the whole series:

Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

On Sat, Jun 13, 2020 at 8:40 AM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Wed, May 27, 2020 at 10:59:55PM -0400, Jason Andryuk wrote:
> > On Mon, May 25, 2020 at 6:36 PM Jason Andryuk <jandryuk@gmail.com> wrote:
> > >
> > > On Sun, May 24, 2020 at 10:50 PM Jason Andryuk <jandryuk@gmail.com> wrote:
> > > >
> > > > This series addresses some Coverity reports.  To handle closing FDs, a
> > > > state struct is introduced to track FDs closed in both main() and
> > > > data_loop().
> > >
> > > I've realized the changes here are insufficient to handle the FD
> > > leaks.  That is, the accept()-ed FDs need to be closed inside the for
> > > loop so they aren't leaked with each iteration.  I'll re-work for a
> > > v2.
> >
> > So it turns out this series doesn't leak FDs in the for loop.  FDs are
> > necessarily closed down in data_loop() when the read() returns 0.  The
> > only returns from data_loop() are after the FDs have been closed.
> > data_loop() and some of the functions it calls will call exit(1) on
> > error, but that won't leak FDs.
> >
> > Please review this series.  Sorry for the confusion.
>
> For the whole series:
>
> Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Thanks for the review.  Sorry for forgetting to CC you on this series
and the v2 posted on Jun 10th as well.  For v2, patch 1 now also
changes strncpy to strcpy to avoid a gcc-10 warning reported by Olaf
Hering.  Patches 2 & 3 are new to move some perror calls.  v1 patches
2-8 shifted to 4-10 in v2, but are unchanged.

Thanks,
Jason