Re: [Qemu-devel] [PATCH v1] block/vxhs: Add Veritas HyperScale VxHS block device support

[Markus Armbruster]

Jeff Cody <address@hidden> writes:

> On Wed, Oct 26, 2016 at 09:33:30PM +0000, Buddhi Madhav wrote:
>> 
>> 
>> On 10/25/16, 9:41 PM, "Jeff Cody" <address@hidden> wrote:
>> 
>> >On Tue, Oct 25, 2016 at 03:02:07PM -0700, Ashish Mittal wrote:
>> >> This patch adds support for a new block device type called "vxhs".
>> >> Source code for the library that this code loads can be downloaded from:
>> >> https://github.com/MittalAshish/libqnio.git
>> >>
>> >
>> >I grabbed the latest of libqnio, compiled it (had to disable -Werror), and
>> >tried it out.  I was able to do a qemu-img info on a raw file, but it
>> >would
>> >just hang when trying a format such as qcow2.  I am assuming
>> >this is a limitation of test_server, and not libqnio.
>> 
>> On my build I did not get any build errors.
>> 
>
> Likely a difference in gcc versions; I am compiling with gcc 6.2.1.  Here
> are the warnings I get (this is just for your benefit, not really part of
> this driver review):
>
> lib/qnio/cJSON.c: In function ‘cJSON_strcasecmp’:
> lib/qnio/cJSON.c:41:5: warning: this ‘if’ clause does not guard... 
> [-Wmisleading-indentation]
>      if (!s1) return (s1==s2)?0:1;if (!s2) return 1;
>      ^~
> lib/qnio/cJSON.c:41:34: note: ...this statement, but the latter is 
> misleadingly indented as if it is guarded by the ‘if’
>      if (!s1) return (s1==s2)?0:1;if (!s2) return 1;
>                                   ^~
> lib/qnio/cJSON.c: In function ‘print_object’:
> lib/qnio/cJSON.c:440:9: warning: this ‘if’ clause does not guard... 
> [-Wmisleading-indentation]
>          if (fmt) *ptr++='\n';*ptr=0;
>          ^~
> lib/qnio/cJSON.c:440:30: note: ...this statement, but the latter is 
> misleadingly indented as if it is guarded by the ‘if’
>          if (fmt) *ptr++='\n';*ptr=0;
>                               ^
> lib/qnio/cJSON.c: In function ‘cJSON_DetachItemFromArray’:
> lib/qnio/cJSON.c:467:5: warning: this ‘if’ clause does not guard... 
> [-Wmisleading-indentation]
>      if (c->prev) c->prev->next=c->next;if (c->next) c->next->prev=c->prev;if 
> (c==array->child) array->child=c->next;c->prev=c->next=0;return c;}
>      ^~
> lib/qnio/cJSON.c:467:40: note: ...this statement, but the latter is 
> misleadingly indented as if it is guarded by the ‘if’
>      if (c->prev) c->prev->next=c->next;if (c->next) c->next->prev=c->prev;if 
> (c==array->child) array->child=c->next;c->prev=c->next=0;return c;}

Looks like this code hasn't been competently reviewed, not even fed to a
state-of-the-art static analyzer.  As long as that remains the case,
linking with it outside experimental settings feels unadvisable.

[...]