Re: [PATCH v1 3/3] blktap2: Silence warnings under GCC 5.1.1

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Oct 05, 2015 at 08:46:03AM -0600, Jan Beulich wrote:
> >>> On 05.10.15 at 16:26, <konrad.wilk@oracle.com> wrote:
> > On Mon, Oct 05, 2015 at 04:46:55AM -0600, Jan Beulich wrote:
> >> >>> On 05.10.15 at 10:49, <andrew.cooper3@citrix.com> wrote:
> >> > On 03/10/15 19:39, Konrad Rzeszutek Wilk wrote:
> >> >> I get compile warnings telling me that
> >> >>    s->connections[i].fd == fd
> >> >>
> >> >> 'i' may be past the array. Adding in an extra condition
> >> >> on the loop fixes that.
> >> >>
> >> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> > 
> >> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> > 
> >> > Furthermore, I can't see any logic which prevents s->connected getting
> >> > larger than MAX_CONNECTIONS
> >> 
> >> Iirc Olaf had already suggested such a patch quite some time ago,
> >> and not having seen the point back then I also don't see the point
> >> now: ctl_accept() prevents ->connected from growing beyond 1,
> >> and there's no other place where the value could get incremented.
> > 
> > Thoughts on what to do about the compiler warnings which make this
> > compiler errors (since we compile with -Werror) and one can't
> > compile Xen? A different fix (make s->connected not be an array?)
> 
> To be honest I think this needs to be taken care of at the compiler
> side, as I can't see any reason for the warning. The workaround
> therefore would be to suppress the warning via -Wno-* until the
> compiler side would get fixed. (Of course I'm open to be convinced
> otherwise, i.e. this not being a compiler issue.)

There is an code issue here as well.

Andrew said  - there is no logic that "prevents s->connected getting larger
than MAX_CONNECTIONS." which would resulted in accessing s->connections
outside its array size.

Let me look to see if I can add some s->connected boundaries. Thought
this is going low on my list of priorities.

> 
> >> Also, Konrad, regarding the subject (since this repeats from an
> >> earlier patch of yours) - why do you reference a specific, non-
> >> release version of gcc? Why not simply say 5.x? Because if the
> >> problem is indeed only present in a non-release version, I don't
> >> think we should bother working around such issues.
> > 
> > I just ran 'gcc --version' and that is what it spit out. Since
> > it is part of an official Fedora release I figured it is 'released'
> > in some way?
> 
> Well, if my understanding of gcc's new versioning is correct, only
> <x>.<y>.<z> with y > 0 and z == 0 are released versions. 5.1.1
> would be an RC for 5.2.0 (which already got released).

And indeed: gcc (GCC) 5.1.1 20150618 (Red Hat 5.1.1-4)
while 5.2.0 was out on July 16 (0716). And doing yum upgrade gcc
gives me no new code, hence it seems that Fedora Core 22 is stuck
at an RC for 5.2.0.

> 
> Jan
>