Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

> > > > +                }else if ( !strcmp(optkey, "rdm_policy") ) {
> > > > +                    if ( !strcmp(tok, "strict") ) {
> > > > +                        pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
> > > > +                    } else if ( !strcmp(tok, "relaxed") ) {
> > > > +                        pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
> > > > +                    } else {
> > > > +                        XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM property"
> > > > +                                          " policy: 'strict' or 'relaxed'.",
> > > > +                                     tok);
> > > > +                        goto parse_error;
> > > > +                    }
> > > This section has coding style (whitespace) problems and long lines.
> > > If you need to respin, please fix them.
> > Are you saying this?
> >
> > } else if (  -> }else if (
> > } else { -> }else {
> Also spurious spaces inside brackets.  Please see CODING_STYLE.
I still can't understand what I'm missing here after compared to other 
contexts inside xlu_pci_parse_bdf(). So I have to paste this entirely,
                }else if ( !strcmp(optkey, "rdm_policy") ) {
                    if ( !strcmp(tok, "strict") ) {
                        pcidev->rdm_policy = 
LIBXL_RDM_RESERVE_POLICY_STRICT;
                    }else if ( !strcmp(tok, "relaxed") ) {
                        pcidev->rdm_policy = 
LIBXL_RDM_RESERVE_POLICY_RELAXED;
                    }else{
                        XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM 
property"
                                          " policy: 'strict' or 
'relaxed'.",
                                     tok);
                        goto parse_error;
                    }
                }else{

This is not a long code segment, so could you point them just one by one?

> > Additionally I don't found which line is over 80 characters.
[snip]

> > > Really I would prefer that this parsing was done with a miniature flex
> > > parser, rather than ad-hoc pointer arithmetic and use of strtok.
> > Sorry, could you show this explicitly?
> Something like what was done for disk devices.  See libxlu_disk_l.l
> for an example.  In this case your code would be a lot less
> complicated than what you see there.
>
> After the codefreeze I would probably have some time to write it for
Sounds yourself would do this so currently I just keep the original, right?

Thanks
Tiejun

> you.  (I think that would be valuable because libxlu_disk_l.l is a
> very complicated example, and I want be able to point future
> submitters at something simpler.)
>
> Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel