[Libvirt-cim] [PATCH 2/4] FilterEntry: Fix endianness issues
Viktor Mihajlovski
mihajlov at linux.vnet.ibm.com
Wed Nov 13 16:51:20 UTC 2013
On 11/12/2013 10:28 PM, John Ferlan wrote:
> On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
>> From: Thilo Boehm <tboehm at linux.vnet.ibm.com>
>>
>> A number of CIM properties was set in an endianness-unsafe manner
>> leading to failures on big endian systems.
>>
>> Signed-off-by: Thilo Boehm <tboehm at linux.vnet.ibm.com>
>> Signed-off-by: Viktor Mihajlovski <mihajlov at linux.vnet.ibm.com>
>> ---
>> src/Virt_FilterEntry.c | 46 ++++++++++++++++++++++++----------------------
>> 1 file changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c
>> index b7042da..e41b4b6 100644
>> --- a/src/Virt_FilterEntry.c
>> +++ b/src/Virt_FilterEntry.c
>> @@ -59,8 +59,8 @@ struct rule_data_t {
>> const char *dstportend;
>> };
>>
>> -static int octets_from_mac(const char * s, unsigned int *buffer,
>> - unsigned int size)
>> +static int octets_from_mac(const char * s, uint8_t *buffer,
>> + unsigned int size)
>> {
>> unsigned int _buffer[6];
>> unsigned int i, n = 0;
>> @@ -86,8 +86,8 @@ static int octets_from_mac(const char * s, unsigned int *buffer,
>> return n;
>> }
>>
>> -static int octets_from_ip(const char * s, unsigned int *buffer,
>> - unsigned int size)
>> +static int octets_from_ip(const char * s, uint8_t *buffer,
>> + unsigned int size)
>> {
>> struct in6_addr addr;
>> unsigned int family = 0;
>> @@ -116,7 +116,8 @@ static int octets_from_ip(const char * s, unsigned int *buffer,
>> return n;
>> }
>>
>> -static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, unsigned int *bytes, int size)
>> +static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, uint8_t *bytes,
>> + int size)
>> {
>> CMPIStatus s = {CMPI_RC_OK, NULL};
>> CMPIArray *array = NULL;
>> @@ -216,7 +217,7 @@ static int convert_action(const char *s)
>> return action;
>> }
>>
>> -static unsigned long convert_protocol_id(const char *s)
>> +static uint16_t convert_protocol_id(const char *s)
>> {
>> enum {NONE = 0, IPV4 = 2048, ARP = 2054, RARP = 32821, IPV6 = 34525} id = NONE;
>>
>> @@ -239,7 +240,7 @@ static void convert_mac_rule_to_instance(
>> CMPIInstance *inst,
>> const CMPIBroker *broker)
>> {
>> - unsigned int bytes[48];
>> + uint8_t bytes[48];
>> unsigned int size = 0;
>> CMPIArray *array = NULL;
>>
>> @@ -280,7 +281,7 @@ static void convert_mac_rule_to_instance(
>> (CMPIValue *)&array, CMPI_uint8A);
>>
>> if (rule->var.mac.protocol_id != NULL) {
>> - unsigned long n = convert_protocol_id(rule->var.mac.protocol_id);
>> + uint16_t n = convert_protocol_id(rule->var.mac.protocol_id);
>>
>> /* Unknown protocolid string. Try converting from hexadecimal value */
>> if (n == 0)
>> @@ -366,18 +367,19 @@ static void convert_ip_rule_to_instance(
>> CMPIInstance *inst,
>> const CMPIBroker *broker)
>> {
>> - unsigned int bytes[48];
>> + uint8_t bytes[48];
>> unsigned int size = 0;
>> - unsigned int n = 0;
>> + uint8_t ipver_num = 0;
>> + uint16_t port_num = 0;
>> CMPIArray *array = NULL;
>> struct rule_data_t rule_data;
>>
>> if (strstr(rule->protocol_id, "v6"))
>> - n = 6;
>> + ipver_num = 6;
>> else
>> - n = 4;
>> + ipver_num = 4;
>>
>> - CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8);
>> + CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&ipver_num, CMPI_uint8);
>>
>> fill_rule_data(rule, &rule_data);
>>
>> @@ -480,27 +482,27 @@ static void convert_ip_rule_to_instance(
>> }
>>
>> if (rule_data.srcportstart) {
>> - n = atoi(rule_data.srcportstart);
>> + port_num = atoi(rule_data.srcportstart);
>
> Hmm.. technically atoi() returns an 'int'... In libvirt code this would
> be a call to a function which does a strtoul() and makes sure there's no
> overflow, etc. However, since this code is full of other situations
> such as this, I'm "OK" with the way it's done...
too true ... we have tried to restrict the patch to the big/little-
endian fix and not to mix in cleanups.
>
>
>> CMSetProperty(inst, "HdrSrcPortStart",
>> - (CMPIValue *)&n, CMPI_uint16);
>> + (CMPIValue *)&port_num, CMPI_uint16);
>> }
>>
>> if (rule_data.srcportend) {
>> - n = atoi(rule_data.srcportend);
>> + port_num = atoi(rule_data.srcportend);
>> CMSetProperty(inst, "HdrSrcPortEnd",
>> - (CMPIValue *)&n, CMPI_uint16);
>> + (CMPIValue *)&port_num, CMPI_uint16);
>> }
>>
>> if (rule_data.dstportstart) {
>> - n = atoi(rule_data.dstportstart);
>> + port_num = atoi(rule_data.dstportstart);
>> CMSetProperty(inst, "HdrDestPortStart",
>> - (CMPIValue *)&n, CMPI_uint16);
>> + (CMPIValue *)&port_num, CMPI_uint16);
>> }
>>
>> if (rule_data.dstportend) {
>> - n = atoi(rule_data.dstportend);
>> + port_num = atoi(rule_data.dstportend);
>> CMSetProperty(inst, "HdrDestPortEnd",
>> - (CMPIValue *)&n, CMPI_uint16);
>> + (CMPIValue *)&port_num, CMPI_uint16);
>> }
>> }
>>
>> @@ -515,7 +517,7 @@ static CMPIInstance *convert_rule_to_instance(
>> const char *sys_name = NULL;
>> const char *sys_ccname = NULL;
>> const char *basename = NULL;
>> - int action, direction, priority = 0;
>> + uint16_t action, direction, priority = 0;
>
> This is declared an uint16_t here; however, later on it's stored as a
> signed int16:
>
> priority = convert_priority(rule->priority);
> CMSetProperty(inst, "Priority", (CMPIValue *)&priority,
> CMPI_sint16);
hm ... collateral damage. The "Priority" property is indeed a sint16
(schema/FilterEntry.mof), "Direction" and "Action" are uint16's.
>
>
> Also in convert_filter_to_instance(), there's some similar oddities:
>
> ...
> int direction = 0, priority;
oversight ... should be
uint16_t direction = 0;
int16_t priority;
> ...
> CMSetProperty(inst, "Direction", (CMPIValue *)&direction,
> CMPI_uint16);
>
> priority = convert_priority(filter->priority);
further, we need to change the return type of all the conver_xxx
functions ...
In my opinion this warrants a V2 patch ... do you prefer a
resend of entire series or just this single patch?
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
More information about the Libvirt-cim
mailing list