[PATCH v2] blktap2: update connection handling to fix build with gcc5

[PATCH v2] blktap2: update connection handling to fix build with gcc5

[Olaf Hering]

blktap2 fails to build with gcc5 because it fails to recognize that
there can be just one active connection (enforced in ctl_accept).

Add a hack to use MAX_CONNECTIONS instead of ARRAY_SIZE:
gives a smaller patch, and the code does not handle holes in connections[] anyway.

[  198s] block-log.c: In function 'ctl_close_sock':
[  198s] block-log.c:363:23: error: array subscript is above array bounds [-Werror=array-bounds]
[  198s]      if (s->connections[i].fd == fd) {
[  198s]                        ^
[  198s] block-log.c: In function 'ctl_request':
[  198s] block-log.c:549:23: error: array subscript is above array bounds [-Werror=array-bounds]
[  198s]      if (s->connections[i].id == id)
[  198s]                        ^
[  198s] cc1: all warnings being treated as errors
[  198s] /home/abuild/rpmbuild/BUILD/xen-4.6.31382/non-dbg/tools/blktap2/drivers/../../../tools/Rules.mk:107: recipe for target 'block-log.o' failed
[  198s] make[5]: *** [block-log.o] Error 1

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/blktap2/drivers/block-log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/blktap2/drivers/block-log.c b/tools/blktap2/drivers/block-log.c
index 5330cdc..1934227 100644
--- a/tools/blktap2/drivers/block-log.c
+++ b/tools/blktap2/drivers/block-log.c
@@ -359,7 +359,7 @@ static int ctl_close_sock(struct tdlog_state* s, int fd)
 {
   int i;
 
-  for (i = 0; i < s->connected; i++) {
+  for (i = 0; i < s->connected && i < MAX_CONNECTIONS; i++) {
     if (s->connections[i].fd == fd) {
       tapdisk_server_unregister_event(s->connections[i].id);
       close(s->connections[i].fd);
@@ -545,7 +545,7 @@ static inline int ctl_find_connection(struct tdlog_state *s, event_id_t id)
 {
   int i;
 
-  for (i = 0; i < s->connected; i++)
+  for (i = 0; i < s->connected && i < MAX_CONNECTIONS; i++)
     if (s->connections[i].id == id)
       return s->connections[i].fd;

Re: [PATCH v2] blktap2: update connection handling to fix build with gcc5

[Jan Beulich]

>>> On 21.07.15 at 10:42, <olaf@aepfle.de> wrote:
> --- a/tools/blktap2/drivers/block-log.c
> +++ b/tools/blktap2/drivers/block-log.c
> @@ -359,7 +359,7 @@ static int ctl_close_sock(struct tdlog_state* s, int fd)
>  {
>    int i;
>  
> -  for (i = 0; i < s->connected; i++) {
> +  for (i = 0; i < s->connected && i < MAX_CONNECTIONS; i++) {
>      if (s->connections[i].fd == fd) {
>        tapdisk_server_unregister_event(s->connections[i].id);
>        close(s->connections[i].fd);
> @@ -545,7 +545,7 @@ static inline int ctl_find_connection(struct tdlog_state *s, event_id_t id)
>  {
>    int i;
>  
> -  for (i = 0; i < s->connected; i++)
> +  for (i = 0; i < s->connected && i < MAX_CONNECTIONS; i++)
>      if (s->connections[i].id == id)
>        return s->connections[i].fd;
>  

Which now makes the reader (wrongly) imply that s->connected
might be equal to or larger than MAX_CONNECTIONS.

Jan

Re: [PATCH v2] blktap2: update connection handling to fix build with gcc5

[Ian Campbell]

On Tue, 2015-07-21 at 02:48 -0600, Jan Beulich wrote:
> >>> On 21.07.15 at 10:42, <olaf@aepfle.de> wrote:
> > --- a/tools/blktap2/drivers/block-log.c
> > +++ b/tools/blktap2/drivers/block-log.c
> > @@ -359,7 +359,7 @@ static int ctl_close_sock(struct tdlog_state* s, int fd)
> >  {
> >    int i;
> >  
> > -  for (i = 0; i < s->connected; i++) {
> > +  for (i = 0; i < s->connected && i < MAX_CONNECTIONS; i++) {
> >      if (s->connections[i].fd == fd) {
> >        tapdisk_server_unregister_event(s->connections[i].id);
> >        close(s->connections[i].fd);
> > @@ -545,7 +545,7 @@ static inline int ctl_find_connection(struct tdlog_state *s, event_id_t id)
> >  {
> >    int i;
> >  
> > -  for (i = 0; i < s->connected; i++)
> > +  for (i = 0; i < s->connected && i < MAX_CONNECTIONS; i++)
> >      if (s->connections[i].id == id)
> >        return s->connections[i].fd;
> >  
> 
> Which now makes the reader (wrongly) imply that s->connected
> might be equal to or larger than MAX_CONNECTIONS.

I was just thinking that perhaps either an assert() or an:
        if ( s->connected >= MAX_CONNECTIONS ) {
        	log
        	return error
        }
might be more appropriate.

Ian.