On 03/27/2015 06:01 PM, Eric Blake wrote:
> On 03/26/2015 12:42 PM, Laine Stump wrote:
>> netcf has always supported multiple IPv6 addresses, but only a single
>> IPv4 address. This patch remedies that, at least for the redhat driver
>> (also used for Fedora and CentOS).
>>
>> I can't claim that the xsl transforms in redhat-(get|put).xsl are the
>> optimum code, but they do get the job done (no mean feat, since this
>> is the first time I've ever made anything beyond a cosmetic change to
>> an xsl script).
> And I'm no xsl transform expert either, so take my review with a grain
> of salt.
>
>
>> +++ b/data/xml/redhat-get.xsl
>> @@ -192,10 +192,18 @@
>> </xsl:when>
>> <xsl:when test="ip">
>> <node label="BOOTPROTO" value="none"/>
>> - <node label="IPADDR" value="{ip/@address}"/>
>> - <xsl:if test="ip/@prefix">
>> - <node label="NETMASK" value="{ipcalc:netmask(ip/@prefix)}"/>
>> - </xsl:if>
>> + <xsl:for-each select="ip">
>> + <xsl:if test="position() = 1">
>> + <xsl:call-template name="ipv4-address">
>> + <xsl:with-param name="index"/>
>> + </xsl:call-template>
>> + </xsl:if>
>> + <xsl:if test="position() > 1 and position() < 100">
> I think you have an off-by-one:
>
> 1 vs. 2-99...
Ah, right. All of this indexing starting at 0 instead of 1 is really a
pain for an old C-head like me.
Note that the xsl:if just above that for the case of position() = 1 is
there so that the first address will be IPADDR rather than IPADDR0 -
it's essentially calling ipv4-address with index = "" (that's another
thing I could only learn by trial and error - you can't directly set a
param to "", you have to just not set it at all). Those from 2 - 100
should be called with index = 1 - 99 , but as you've pointed out, the
last time I'll do it is for position() == 99, or index = 99 - 1 = 98.
Thanks for catching that!
>
>
>> +++ b/data/xml/redhat-put.xsl
>> + <xsl:for-each select="node[substring(@label, 1, 6) = 'IPADDR']">
>> + <xsl:variable name="index" select="substring(@label, 7, 3)"/>
>> + <xsl:if test="number($index) > 0 and number($index) <
>> 100">
> compared to 0 vs. 1-99.
>
> That is, if position() is 1-based, you want <= 100, or < 101, as your
> terminating condition (operating on IPADDR[position()-1] for each
> iteration of the for-each).
>
> If you fix or explain the difference, then the rest of the patch looks
> okay for an ACK.
>
Okay. I'll change the index before pushing.
Endless thanks for all the reviews.
_______________________________________________
netcf-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/netcf-devel
