[Libguestfs] [PATCH libnbd] copy: Refactor ‘struct rw’.

[Nir Soffer]

On Mon, Feb 22, 2021 at 7:25 PM Richard W.M. Jones <rjones at redhat.com> wrote:
>
> On Mon, Feb 22, 2021 at 07:06:39PM +0200, Nir Soffer wrote:
> > On Mon, Feb 22, 2021 at 5:42 PM Richard W.M. Jones <rjones at redhat.com> wrote:
> > >
> > > Make this a (fairly) abstract structure.  At least hide the subtype
> > > fields from the main program.  This change is pure refactoring and
> > > doesn’t change the semantics.
> >
> > Nicer this way, although a little less type safe.
>
> > > +  return (struct rw *) rwf;
> >
> > We can avoid the cast here by returning &(rwf->rw)
>
> I'll make that change (and elsewhere too).
>
> > > -static int
> > > -open_local (const char *prog,
> > > -            const char *filename, bool writing, struct rw *rw)
> > > +static struct rw *
> > > +open_local (const char *filename, bool writing)
> > >  {
> > >    int flags, fd;
> > > +  struct stat stat;
> > >
> > >    if (strcmp (filename, "-") == 0) {
> > >      synchronous = true;
> > >      fd = writing ? STDOUT_FILENO : STDIN_FILENO;
> > >      if (writing && isatty (fd)) {
> > > -      fprintf (stderr, "%s: refusing to write to tty\n", prog);
> > > +      fprintf (stderr, "%s: refusing to write to tty\n", "nbdcopy");
> >
> > Is it intended to replace prog with "nbdcopy"?
>
> The change is a bit messed up - I think I may have moved this code
> around twice.  But even better would be to use a getprogname wrapper
> (a la gnulib).
>
> > I looked at it briefly, this is a large change, but generally it
> > looks good.
>
> OK to push this (with fixes)?

Looks safe enough to me.

> I sent you privately your patch from
> the weekend rebased on top of this one.

Thanks, will test again.