[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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)?  I sent you privately your patch from
the weekend rebased on top of this one.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]