Muli Ben-Yehuda
2005-Apr-19 14:00 UTC
[Xen-devel] [PATCH] grant_table.c PIN_FAIL macro hides flow control
Hi,
grant_table.c''s IN_FAIL macro prints a message, sets rc and then jumps
to a label. This is flow control and should not be hidden in a macro
IMHO[1]. This patch gets rid of the macro, makes the flow control
explicit, and makes further cleanups possible. Patch against today''s
bk tree, compile tested only.
[1] See http://research.microsoft.com/specncheck/docs/engler.pdf, look
for the evilest bug...
Signed-off-by: Muli Ben-Yehuda <mulix@mulix.org>
===== grant_table.c 1.36 vs edited ====--- 1.36/xen/common/grant_table.c
2005-04-19 15:09:04 +03:00
+++ edited/grant_table.c 2005-04-19 16:51:51 +03:00
@@ -30,13 +30,6 @@
#include <xen/shadow.h>
#include <xen/mm.h>
-#define PIN_FAIL(_lbl, _rc, _f, _a...) \
- do { \
- DPRINTK( _f, ## _a ); \
- rc = (_rc); \
- goto _lbl; \
- } while ( 0 )
-
static inline int
get_maptrack_handle(
grant_table_t *t)
@@ -119,9 +112,12 @@
if ( unlikely((sflags & GTF_type_mask) != GTF_permit_access) ||
unlikely(sdom != mapping_d->id) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Bad flags (%x) or dom (%d). (NB. expected dom
%d)\n",
+ {
+ DPRINTK("Bad flags (%x) or dom (%d). (NB. expected dom
%d)\n",
sflags, sdom, mapping_d->id);
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
/* Merge two 16-bit values into a 32-bit combined update. */
/* NB. Endianness! */
@@ -132,24 +128,33 @@
{
new_scombo |= GTF_writing;
if ( unlikely(sflags & GTF_readonly) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Attempt to write-pin a r/o grant
entry.\n");
+ {
+ DPRINTK("Attempt to write-pin a r/o grant
entry.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
}
/* NB. prev_scombo is updated in place to seen value. */
if ( unlikely(cmpxchg_user((u32 *)&sha->flags,
prev_scombo,
new_scombo)) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Fault while modifying shared flags and
domid.\n");
+ {
+ DPRINTK("Fault while modifying shared flags and
domid.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
/* Did the combined update work (did we see what we expected?). */
if ( likely(prev_scombo == scombo) )
break;
if ( retries++ == 4 )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Shared grant entry is unstable.\n");
+ {
+ DPRINTK("Shared grant entry is unstable.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
/* Didn''t see what we expected. Split out the seen flags
& dom. */
/* NB. Endianness! */
@@ -169,8 +174,9 @@
{
clear_bit(_GTF_writing, &sha->flags);
clear_bit(_GTF_reading, &sha->flags);
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Could not pin the granted frame (%lx)!\n",
frame);
+ DPRINTK("Could not pin the granted frame (%lx)!\n",
frame);
+ rc = GNTST_general_error;
+ goto unlock_out;
}
if ( dev_hst_ro_flags & GNTMAP_device_map )
@@ -191,8 +197,11 @@
* A more accurate check cannot be done with a single comparison.
*/
if ( (act->pin & 0x80808080U) != 0 )
- PIN_FAIL(unlock_out, ENOSPC,
- "Risk of counter overflow %08x\n", act->pin);
+ {
+ DPRINTK("Risk of counter overflow %08x\n", act->pin);
+ rc = ENOSPC;
+ goto unlock_out;
+ }
frame = act->frame;
@@ -204,23 +213,32 @@
u16 prev_sflags;
if ( unlikely(sflags & GTF_readonly) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Attempt to write-pin a r/o grant
entry.\n");
+ {
+ DPRINTK("Attempt to write-pin a r/o grant
entry.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
prev_sflags = sflags;
/* NB. prev_sflags is updated in place to seen value. */
if ( unlikely(cmpxchg_user(&sha->flags, prev_sflags,
prev_sflags | GTF_writing)) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Fault while modifying shared flags.\n");
+ {
+ DPRINTK("Fault while modifying shared flags.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
if ( likely(prev_sflags == sflags) )
break;
if ( retries++ == 4 )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Shared grant entry is unstable.\n");
+ {
+ DPRINTK("Shared grant entry is unstable.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
+ }
sflags = prev_sflags;
}
@@ -229,8 +247,9 @@
PGT_writable_page)) )
{
clear_bit(_GTF_writing, &sha->flags);
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Attempt to write-pin a unwritable
page.\n");
+ DPRINTK("Attempt to write-pin a unwritable page.\n");
+ rc = GNTST_general_error;
+ goto unlock_out;
}
}
@@ -520,8 +539,11 @@
else if ( frame == GNTUNMAP_DEV_FROM_VIRT )
{
if ( !( flags & GNTMAP_device_map ) )
- PIN_FAIL(unmap_out, GNTST_bad_dev_addr,
- "Bad frame number: frame not mapped for dev
access.\n");
+ {
+ DPRINTK("Bad frame number: frame not mapped for dev
access.\n");
+ rc = GNTST_bad_dev_addr;
+ goto unmap_out;
+ }
frame = act->frame;
/* Frame will be unmapped for device access below if virt addr okay. */
@@ -529,8 +551,12 @@
else
{
if ( unlikely(frame != act->frame) )
- PIN_FAIL(unmap_out, GNTST_general_error,
- "Bad frame number doesn''t match
gntref.\n");
+ {
+ rc = GNTST_general_error;
+ DPRINTK("Bad frame number doesn''t match
gntref.\n");
+ goto unmap_out;
+ }
+
if ( flags & GNTMAP_device_map )
act->pin -= (flags & GNTMAP_readonly) ? GNTPIN_devr_inc
: GNTPIN_devw_inc;
--
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel