[Libguestfs] [PATCH libnbd 7/9] generator: On entry to API functions, check Flags and OFlags parameters.

[Richard W.M. Jones]

On Sat, Aug 10, 2019 at 04:38:20PM -0500, Eric Blake wrote:
> On 8/10/19 8:02 AM, Richard W.M. Jones wrote:
> > Generate checks that no unknown (at the time of compilation) flags are
> > passed to Flags or OFlags parameters.
> > ---
> >  generator/generator | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> 
> We may still want to introduce a testing mode where you can use libnbd
> to drive unknown flags to a server that understands them, for
> experimenting with protocol extensions not yet included in the NBD
> specification. But if we ever do add such a mode, it shouldn't be hard
> to make this sanity checking conditional on that mode.
> 
> > 
> > diff --git a/generator/generator b/generator/generator
> > index 96d1148..a6aea26 100755
> > --- a/generator/generator
> > +++ b/generator/generator
> > @@ -3689,6 +3689,19 @@ let generate_lib_api_c () =
> >      );
> >  
> >      (* Check parameters are valid. *)
> > +    let print_flags_check n { flag_prefix; flags } =
> > +      let value = match errcode with
> > +        | Some value -> value
> > +        | None -> assert false in
> > +      let mask = List.fold_left (lor) 0 (List.map snd flags) in
> > +      pr "  if ((%s & ~%d) != 0) {\n" n mask;
> > +      pr "    set_error (EINVAL, \"%%s: invalid value for flag: %%d\",\n";
> > +      pr "               \"%s\", %s);\n" n n;
> > +      pr "    ret = %s;\n" value;
> > +      pr "    goto out;\n";
> 
> Some of the checks in lib/rw.c are now unreachable with this in place.
> Is it worth simplifying that?  (But not all of them - there are still
> checks that depend on runtime values, such as nbd_pread accepting _DF
> only if the server advertises it after the client requests structured
> replies).  Also, this lets us pass all four existing command flags to
> all commands that accept an OFlags parameter, even though none of the
> commands accept all flags at once - the real protection being added here
> is the check for completely unrecognized flags.

Right - this doesn't replace fine-grained checks in the functions
themselves.  I'll have a look at lib/rw.c and see what checks might be
removed.

Rich.

> But the changes here look reasonable. ACK.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 




-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html