[bug report] xen-blkback: don't "handle" error by BUG()

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

Hello Jan Beulich,

The patch 5a264285ed1c: "xen-blkback: don't "handle" error by BUG()"
from Feb 15, 2021, leads to the following static checker warning:

        drivers/block/xen-blkback/blkback.c:836 xen_blkbk_map()
        warn: should this be a bitwise negate mask?

drivers/block/xen-blkback/blkback.c
   823           * Now swizzle the MFN in our domain with the MFN from the 
other domain
   824           * so that when we access vaddr(pending_req,i) it has the 
contents of
   825           * the page from the other domain.
   826           */
   827          for (seg_idx = last_map, new_map_idx = 0; seg_idx < map_until; 
seg_idx++) {
   828                  if (!pages[seg_idx]->persistent_gnt) {
   829                          /* This is a newly mapped grant */
   830                          BUG_ON(new_map_idx >= segs_to_map);
   831                          if (unlikely(map[new_map_idx].status != 0)) {
   832                                  pr_debug("invalid buffer -- could not 
remap it\n");
   833                                  gnttab_page_cache_put(&ring->free_pages,
   834                                                        
&pages[seg_idx]->page, 1);
   835                                  pages[seg_idx]->handle = 
BLKBACK_INVALID_HANDLE;
   836                                  ret |= !ret;

Originally this code was:

        ret |= 1;

Now it's equivalent to:

        if (!ret)
                ret = 1;

This is the second user of this idiom in the kernel.  Both were
introduced in the last month so maybe it's a new trend or something...
Anyway.  False positive warning.

But my main question isn't really related to this patch.  What does
the 1 mean in this context?  I always feel like there should be
documentation when functions return a mix of error codes, zero and one
but there isn't any here.

I have reviewed this and so far as I can see setting "ret = 1;" is
always treated exactly the same as an error code by everything.  Every
single place where this is checked just checks for ret is zero.  The
value is propagated two functions back and then it becomes -EIO.

???

   837                                  goto next;
   838                          }
   839                          pages[seg_idx]->handle = 
map[new_map_idx].handle;
   840                  } else {
   841                          continue;
   842                  }
   843                  if (use_persistent_gnts &&

regards,
dan carpenter