From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Chen, Tiejun" <tiejun.chen@intel.com>
Subject: Re: [v7][PATCH 16/16] tools: parse to enable new rdm
	policy parameters
Date: Mon, 13 Jul 2015 17:31:47 +0800
Message-ID: <55A38583.9080204@intel.com>
References: <1436420047-25356-1-git-send-email-tiejun.chen@intel.com>	<1436420047-25356-17-git-send-email-tiejun.chen@intel.com>	<21918.48191.157583.452591@mariner.uk.xensource.com>	<559F60AB.2060402@intel.com>
	<21919.40225.618413.570220@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <21919.40225.618413.570220@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Wei Liu <wei.liu2@citrix.com>, Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

>>>> +                }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.
>