Fix rx_EndCall error precedence

Callers of rx_EndCall in various parts of the code handle errors a bit
differently from each other. The correct way to use rx_EndCall is
almost always some form of:

    code = rx_EndCall(call, code);

This will cause the call to abort with 'code' if the call is not
already aborted, and will return the abort code for the call (or 0 if
the call ended successfully). It is thus impossible for 'code' to
start out with a non-zero value in the code snippet above, and end up
with a value of 0 after the code snippet.

Most code follows this pattern, because this is how the
rxgen-generated client RPC wrappers are written. So for any non-split
Rx call, this is how the error precedence works.

However, some code (mostly for Rx split calls), needs to handle
calling rx_EndCall itself, and some code appears to think it is
possible for rx_EndCall to return 0 when we already had a non-zero
error. Such code tries to ensure that we don't ignore an error we
already got by doing something like this:

    code2 = rx_EndCall(call, code);
    if (code2 && !code) {
        code = code2;
    }

However, this is not correct. If a call gets killed with an abort code
partway through executing an RPC, and the client tries to end the RPC
with e.g. EndRXAFS_FetchData, the client will get an error code of
-451 (RXGEN_CC_UNMARSHAL). The actual error code is in the abort code
for the call, but with the above 'code2' snippet, we can easily return
an error of -451 instead, which will usually get interpreted as some
unknown network-related error.

This can manifest as a problem in the unix client, where if a
FetchData call fails due to, for example, an "idle dead" timeout, we
should result with an error code of RX_CALL_TIMEOUT. But because of
the above issue, we'll instead yield an error of -451, causing the
server to be marked down with the following message:

    afs: Lost contact with file server ... (code -451) ...

So, fix most rx_EndCall callers to follow the 'code = rx_EndCall(call,
code);' pattern. Not all of the changes here are to "wrong" code, but
try to make all of the rx_EndCall call sites look more consistent.
There are a few exceptions to this pattern, which warrant some
variations:

 - A few instances in src/WINNT/afsd/cm_dcache.c do seem to want to
   record the original error before we ran rx_EndCall, instead of
   seeing the rx abort code. We still return the rx_EndCall-returned
   value to the caller, though.

 - Any caller of RXAFS_FetchData* needs to read a 'length' raw from
   the rx split stream. If this fails, we need to abort the call, but
   we don't really have an error code to give to rx_EndCall. Failure
   to read a length indicates that the server is not following
   protocol properly, so give rx_EndCall RX_PROTOCOL_ERROR in these
   instances. The call should already be aborted by this point, so
   most of the time this code will be ignored; it will only make a
   difference if the server tries to end the call successfully without
   sending a length, which is indeed a protocol error.

 - Some Rx clients can encounter a local error they don't want to send
   to the server via an abort, so they just end the call successfully,
   and only use the rx abort code if they don't already have a local
   error. This is in a few places like src/butc/dump.c and
   src/volser/vsprocs.c.

 - Several places don't care what the error from rx_EndCall is, such
   as various call sites in server-side code.

The behavior of the Windows client w.r.t rx_EndCall was changed a bit
into its current behavior in commit
a50fa631cad6919d15721ac2c234ebbdda2b4031 (ticket 125018), which just
appears to be wrong. This was partially reverted by commit
ae7ef5f5b963a5c8ce4110a7352e0010cb6cdbc1 (ticket 125351), but some of
the other call sites were unchanged. The Unix client appears to have
been doing this incorrectly for at least FetchData calls since OpenAFS
1.0.

To make it hopefully more clear that rx_EndCall cannot return 0 if
given a non-zero error code, add an assert to rx_EndCall that asserts
that fact.

FIXES 127001

Change-Id: I10bbfe82b55b509e1930abb6c568edb1efd9fd2f
Reviewed-on: http://gerrit.openafs.org/10788
Reviewed-by: D Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c
index 21f52b5..6da6153 100644 (file)
--- a/src/WINNT/afsd/cm_dcache.c
+++ b/src/WINNT/afsd/cm_dcache.c
@@ -45,7 +45,7 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
      * but the vnode involved may or may not be locked depending on whether
      * or not the CM_BUF_WRITE_SCP_LOCKED flag is set.
      */
-    long code, code1;
+    long code;
     cm_scache_t *scp = vscp;
     afs_int32 nbytes;
     afs_int32 save_nbytes;
@@ -357,19 +357,16 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
             }
         }
 
-        code1 = rx_EndCall(rxcallp, code);
+        /* Prefer rx_EndCall error over StoreData error. Note that this will
+        * never set 'code' to 0 if we passed in a non-zero code. */
+        code = rx_EndCall(rxcallp, code);
 
-        if ((code == RXGEN_OPCODE || code1 == RXGEN_OPCODE) && SERVERHAS64BIT(connp)) {
+        if (code == RXGEN_OPCODE && SERVERHAS64BIT(connp)) {
             SET_SERVERHASNO64BIT(connp);
             qdp = NULL;
             nbytes = save_nbytes;
             goto retry;
         }
-        /* Prefer rx_EndCall error over StoreData error */
-        if (code1 != 0) {
-            osi_Log2(afsd_logp, "rx_EndCall converted 0x%x to 0x%x", code, code1);
-            code = code1;
-        }
     } while (cm_Analyze(connp, userp, reqp, &scp->fid, NULL, 1, &outStatus, &volSync, NULL, NULL, code));
 
     code = cm_MapRPCError(code, reqp);
@@ -454,7 +451,7 @@ long cm_StoreMini(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp)
     AFSStoreStatus inStatus;
     AFSVolSync volSync;
     AFSFid tfid;
-    long code, code1;
+    long code;
     osi_hyper_t truncPos;
     cm_conn_t *connp;
     struct rx_call *rxcallp;
@@ -549,16 +546,12 @@ long cm_StoreMini(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp)
                    osi_Log0(afsd_logp, "EndRXAFS_StoreData SUCCESS");
             }
         }
-        code1 = rx_EndCall(rxcallp, code);
+        code = rx_EndCall(rxcallp, code);
 
-        if ((code == RXGEN_OPCODE || code1 == RXGEN_OPCODE) && SERVERHAS64BIT(connp)) {
+        if (code == RXGEN_OPCODE && SERVERHAS64BIT(connp)) {
             SET_SERVERHASNO64BIT(connp);
             goto retry;
         }
-
-        /* prefer StoreData error over rx_EndCall error */
-        if (code == 0 && code1 != 0)
-            code = code1;
     } while (cm_Analyze(connp, userp, reqp, &scp->fid, NULL, 1, &outStatus, &volSync, NULL, NULL, code));
     code = cm_MapRPCError(code, reqp);
 
@@ -1661,7 +1654,7 @@ cm_CloneStatus(cm_scache_t *scp, cm_user_t *userp, int scp_locked,
 long cm_GetBuffer(cm_scache_t *scp, cm_buf_t *bufp, int *cpffp, cm_user_t *userp,
                   cm_req_t *reqp)
 {
-    long code=0, code1=0;
+    long code=0;
     afs_uint32 nbytes;                 /* bytes in transfer */
     afs_uint32 nbytes_hi = 0;            /* high-order 32 bits of bytes in transfer */
     afs_uint64 length_found = 0;
@@ -1890,6 +1883,8 @@ long cm_GetBuffer(cm_scache_t *scp, cm_buf_t *bufp, int *cpffp, cm_user_t *userp
 
     /* now make the call */
     do {
+       long code1;
+
         code = cm_ConnFromFID(&scp->fid, userp, reqp, &connp);
         if (code)
             continue;
@@ -1914,8 +1909,7 @@ long cm_GetBuffer(cm_scache_t *scp, cm_buf_t *bufp, int *cpffp, cm_user_t *userp
                     nbytes_hi = ntohl(nbytes_hi);
                 } else {
                     nbytes_hi = 0;
-                   code = rx_Error(rxcallp);
-                    code1 = rx_EndCall(rxcallp, code);
+                    code = rx_EndCall(rxcallp, RX_PROTOCOL_ERROR);
                     rxcallp = NULL;
                 }
             }
@@ -2167,6 +2161,7 @@ long cm_GetBuffer(cm_scache_t *scp, cm_buf_t *bufp, int *cpffp, cm_user_t *userp
                 osi_Log1(afsd_logp, "CALL EndRXAFS_FetchData skipped due to error %d", code);
         }
 
+        code1 = code;
         if (rxcallp)
             code1 = rx_EndCall(rxcallp, code);
 
@@ -2182,9 +2177,10 @@ long cm_GetBuffer(cm_scache_t *scp, cm_buf_t *bufp, int *cpffp, cm_user_t *userp
                 scp_locked = 0;
             }
             code = 0;
-        /* Prefer the error value from FetchData over rx_EndCall */
-        } else if (code == 0 && code1 != 0)
+        } else {
+           /* Prefer the error from rx_EndCall over any other error */
             code = code1;
+       }
         osi_Log0(afsd_logp, "CALL FetchData DONE");
 
     } while (cm_Analyze(connp, userp, reqp, &scp->fid, NULL, 0, &afsStatus, &volSync, NULL, NULL, code));
@@ -2237,7 +2233,7 @@ long cm_GetBuffer(cm_scache_t *scp, cm_buf_t *bufp, int *cpffp, cm_user_t *userp
 long cm_GetData(cm_scache_t *scp, osi_hyper_t *offsetp, char *datap, int data_length,
                 int * bytes_readp, cm_user_t *userp, cm_req_t *reqp)
 {
-    long code=0, code1=0;
+    long code=0;
     afs_uint32 nbytes;                 /* bytes in transfer */
     afs_uint32 nbytes_hi = 0;           /* high-order 32 bits of bytes in transfer */
     afs_uint64 length_found = 0;
@@ -2356,6 +2352,8 @@ long cm_GetData(cm_scache_t *scp, osi_hyper_t *offsetp, char *datap, int data_le
 
     /* now make the call */
     do {
+       long code1;
+
         code = cm_ConnFromFID(&scp->fid, userp, reqp, &connp);
         if (code)
             continue;
@@ -2380,8 +2378,7 @@ long cm_GetData(cm_scache_t *scp, osi_hyper_t *offsetp, char *datap, int data_le
                     nbytes_hi = ntohl(nbytes_hi);
                 } else {
                     nbytes_hi = 0;
-                   code = rx_Error(rxcallp);
-                    code1 = rx_EndCall(rxcallp, code);
+                    code = rx_EndCall(rxcallp, RX_PROTOCOL_ERROR);
                     rxcallp = NULL;
                 }
             }
@@ -2554,6 +2551,7 @@ long cm_GetData(cm_scache_t *scp, osi_hyper_t *offsetp, char *datap, int data_le
                 osi_Log1(afsd_logp, "CALL EndRXAFS_FetchData skipped due to error %d", code);
         }
 
+       code1 = code;
         if (rxcallp)
             code1 = rx_EndCall(rxcallp, code);
 
@@ -2569,9 +2567,10 @@ long cm_GetData(cm_scache_t *scp, osi_hyper_t *offsetp, char *datap, int data_le
                 scp_locked = 0;
             }
             code = 0;
-        /* Prefer the error value from FetchData over rx_EndCall */
-        } else if (code == 0 && code1 != 0)
+        } else {
+           /* Prefer the error from rx_EndCall over any other error */
             code = code1;
+       }
         osi_Log0(afsd_logp, "CALL FetchData DONE");
 
     } while (cm_Analyze(connp, userp, reqp, &scp->fid, NULL, 0, &afsStatus, &volSync, NULL, NULL, code));
@@ -2601,7 +2600,7 @@ long cm_GetData(cm_scache_t *scp, osi_hyper_t *offsetp, char *datap, int data_le
 long
 cm_VerifyStoreData(cm_bulkIO_t *biod, cm_scache_t *savedScp)
 {
-    long code=0, code1=0;
+    long code=0;
     afs_uint32 nbytes;                 /* bytes in transfer */
     afs_uint32 nbytes_hi = 0;           /* high-order 32 bits of bytes in transfer */
     afs_uint64 length_found = 0;
@@ -2649,6 +2648,8 @@ cm_VerifyStoreData(cm_bulkIO_t *biod, cm_scache_t *savedScp)
 
     /* now make the call */
     do {
+       long code1;
+
         code = cm_ConnFromFID(&scp->fid, userp, reqp, &connp);
         if (code)
             continue;
@@ -2673,8 +2674,7 @@ cm_VerifyStoreData(cm_bulkIO_t *biod, cm_scache_t *savedScp)
                     nbytes_hi = ntohl(nbytes_hi);
                 } else {
                     nbytes_hi = 0;
-                   code = rx_Error(rxcallp);
-                    code1 = rx_EndCall(rxcallp, code);
+                    code = rx_EndCall(rxcallp, RX_PROTOCOL_ERROR);
                     rxcallp = NULL;
                 }
             }
@@ -2780,6 +2780,7 @@ cm_VerifyStoreData(cm_bulkIO_t *biod, cm_scache_t *savedScp)
                 osi_Log1(afsd_logp, "CALL EndRXAFS_FetchData skipped due to error %d", code);
         }
 
+       code1 = code;
         if (rxcallp)
             code1 = rx_EndCall(rxcallp, code);
 
@@ -2795,9 +2796,10 @@ cm_VerifyStoreData(cm_bulkIO_t *biod, cm_scache_t *savedScp)
                 scp_locked = 0;
             }
             code = 0;
-        /* Prefer the error value from FetchData over rx_EndCall */
-        } else if (code == 0 && code1 != 0)
+        } else {
+           /* Prefer the error from rx_EndCall over any other error */
             code = code1;
+       }
         osi_Log0(afsd_logp, "CALL FetchData DONE");
 
     } while (cm_Analyze(connp, userp, reqp, &scp->fid, NULL, 0, &afsStatus, &volSync, NULL, NULL, code));

diff --git a/src/WINNT/afsd/cm_direct.c b/src/WINNT/afsd/cm_direct.c
index 5b9d3a2..491ef44 100644 (file)
--- a/src/WINNT/afsd/cm_direct.c
+++ b/src/WINNT/afsd/cm_direct.c
@@ -63,7 +63,7 @@ int_DirectWrite( IN cm_scache_t *scp,
                  IN void        *memoryRegionp,
                  OUT afs_uint32 *bytesWritten)
 {
-    long code, code1;
+    long code;
     long temp;
     AFSFetchStatus outStatus;
     AFSStoreStatus inStatus;
@@ -178,16 +178,12 @@ int_DirectWrite( IN cm_scache_t *scp,
             }
         }
 
-        code1 = rx_EndCall(rxcallp, code);
+        code = rx_EndCall(rxcallp, code);
 
-        if ((code == RXGEN_OPCODE || code1 == RXGEN_OPCODE) && SERVERHAS64BIT(connp)) {
+        if (code == RXGEN_OPCODE && SERVERHAS64BIT(connp)) {
             SET_SERVERHASNO64BIT(connp);
             goto retry;
         }
-
-        /* Prefer StoreData error over rx_EndCall error */
-        if (code1 != 0)
-            code = code1;
     } while (cm_Analyze(connp, userp, reqp, &scp->fid, NULL, 1, &outStatus, &volSync, NULL, NULL, code));
 
     code = cm_MapRPCError(code, reqp);

diff --git a/src/afs/afs_bypasscache.c b/src/afs/afs_bypasscache.c
index 2e36d00..508ccfc 100644 (file)
--- a/src/afs/afs_bypasscache.c
+++ b/src/afs/afs_bypasscache.c
@@ -596,9 +596,8 @@ afs_PrefetchNoCache(struct vcache *avc,
 
                    if (bytes != sizeof(afs_int32)) {
                        length_hi = 0;
-                       code = rx_Error(tcall);
                        COND_GUNLOCK(locked);
-                       code = rx_EndCall(tcall, code);
+                       code = rx_EndCall(tcall, RX_PROTOCOL_ERROR);
                        COND_RE_GLOCK(locked);
                        tcall = NULL;
                    }

diff --git a/src/afs/afs_fetchstore.c b/src/afs/afs_fetchstore.c
index 99f9c51..3dc865b 100644 (file)
--- a/src/afs/afs_fetchstore.c
+++ b/src/afs/afs_fetchstore.c
@@ -240,18 +240,15 @@ rxfs_storeClose(void *r, struct AFSFetchStatus *OutStatus, int *doProcessFS)
 }
 
 afs_int32
-rxfs_storeDestroy(void **r, afs_int32 error)
+rxfs_storeDestroy(void **r, afs_int32 code)
 {
-    afs_int32 code = error;
     struct rxfs_storeVariables *v = (struct rxfs_storeVariables *)*r;
 
     *r = NULL;
     if (v->call) {
        RX_AFS_GUNLOCK();
-       code = rx_EndCall(v->call, error);
+       code = rx_EndCall(v->call, code);
        RX_AFS_GLOCK();
-       if (!code && error)
-           code = error;
     }
     if (v->tbuffer)
        osi_FreeLargeSpace(v->tbuffer);
@@ -811,7 +808,7 @@ afs_int32
 rxfs_fetchClose(void *r, struct vcache *avc, struct dcache * adc,
                struct afs_FetchOutput *o)
 {
-    afs_int32 code, code1 = 0;
+    afs_int32 code;
     struct rxfs_fetchVariables *v = (struct rxfs_fetchVariables *)r;
 
     if (!v->call)
@@ -826,10 +823,8 @@ rxfs_fetchClose(void *r, struct vcache *avc, struct dcache * adc,
 #endif
         code = EndRXAFS_FetchData(v->call, &o->OutStatus, &o->CallBack,
                                &o->tsync);
-    code1 = rx_EndCall(v->call, code);
+    code = rx_EndCall(v->call, code);
     RX_AFS_GLOCK();
-    if (!code && code1)
-       code = code1;
 
     v->call = NULL;
 
@@ -837,18 +832,15 @@ rxfs_fetchClose(void *r, struct vcache *avc, struct dcache * adc,
 }
 
 afs_int32
-rxfs_fetchDestroy(void **r, afs_int32 error)
+rxfs_fetchDestroy(void **r, afs_int32 code)
 {
-    afs_int32 code = error;
     struct rxfs_fetchVariables *v = (struct rxfs_fetchVariables *)*r;
 
     *r = NULL;
     if (v->call) {
         RX_AFS_GUNLOCK();
-       code = rx_EndCall(v->call, error);
+       code = rx_EndCall(v->call, code);
         RX_AFS_GLOCK();
-       if (error)
-           code = error;
     }
     if (v->tbuffer)
        osi_FreeLargeSpace(v->tbuffer);
@@ -914,7 +906,7 @@ rxfs_fetchInit(struct afs_conn *tc, struct rx_connection *rxconn,
               struct osi_file *fP, struct fetchOps **ops, void **rock)
 {
     struct rxfs_fetchVariables *v;
-    int code = 0, code1 = 0;
+    int code = 0;
 #ifdef AFS_64BIT_CLIENT
     afs_uint32 length_hi = 0;
 #endif
@@ -948,9 +940,8 @@ rxfs_fetchInit(struct afs_conn *tc, struct rx_connection *rxconn,
                if (bytes == sizeof(afs_int32)) {
                    length_hi = ntohl(length_hi);
                } else {
-                   code = rx_Error(v->call);
                    RX_AFS_GUNLOCK();
-                   code1 = rx_EndCall(v->call, code);
+                   code = rx_EndCall(v->call, RX_PROTOCOL_ERROR);
                    RX_AFS_GLOCK();
                    v->call = NULL;
                }
@@ -982,8 +973,7 @@ rxfs_fetchInit(struct afs_conn *tc, struct rx_connection *rxconn,
                length = ntohl(length);
            else {
                RX_AFS_GUNLOCK();
-               code = rx_Error(v->call);
-                code1 = rx_EndCall(v->call, code);
+                code = rx_EndCall(v->call, RX_PROTOCOL_ERROR);
                v->call = NULL;
                length = 0;
                RX_AFS_GLOCK();
@@ -1009,8 +999,7 @@ rxfs_fetchInit(struct afs_conn *tc, struct rx_connection *rxconn,
            if (bytes == sizeof(afs_int32)) {
                 *alength = ntohl(length);
            } else {
-               code = rx_Error(v->call);
-                code1 = rx_EndCall(v->call, code);
+                code = rx_EndCall(v->call, RX_PROTOCOL_ERROR);
                v->call = NULL;
            }
        }
@@ -1026,17 +1015,13 @@ rxfs_fetchInit(struct afs_conn *tc, struct rx_connection *rxconn,
         * requested. It shouldn't do that, and accepting that much data
         * can make us take up more cache space than we're supposed to,
         * so error. */
-       code = rx_Error(v->call);
        RX_AFS_GUNLOCK();
-       code1 = rx_EndCall(v->call, code);
+       code = rx_EndCall(v->call, RX_PROTOCOL_ERROR);
        RX_AFS_GLOCK();
        v->call = NULL;
        code = EIO;
     }
 
-    if (!code && code1)
-       code = code1;
-
     if (code) {
        osi_FreeSmallSpace(v);
         return code;

diff --git a/src/bozo/bos.c b/src/bozo/bos.c
index 3edae25..816853f 100644 (file)
--- a/src/bozo/bos.c
+++ b/src/bozo/bos.c
@@ -1207,7 +1207,7 @@ DoSalvage(struct rx_connection * aconn, char * aparm1, char * aparm2,
        code =
            StartBOZO_GetLog(tcall, AFSDIR_CANONICAL_SERVER_SLVGLOG_FILEPATH);
        if (code) {
-           rx_EndCall(tcall, code);
+           code = rx_EndCall(tcall, code);
            goto done;
        }
        /* copy data */
@@ -1243,7 +1243,7 @@ GetLogCmd(struct cmd_syndesc *as, void *arock)
     tcall = rx_NewCall(tconn);
     code = StartBOZO_GetLog(tcall, as->parms[1].items->data);
     if (code) {
-       rx_EndCall(tcall, code);
+       code = rx_EndCall(tcall, code);
        goto done;
     }
     /* copy data */

diff --git a/src/libadmin/bos/afs_bosAdmin.c b/src/libadmin/bos/afs_bosAdmin.c
index 0d7cb38..da37919 100644 (file)
--- a/src/libadmin/bos/afs_bosAdmin.c
+++ b/src/libadmin/bos/afs_bosAdmin.c
@@ -2689,7 +2689,7 @@ bos_ExecutableCreate(const void *serverHandle, const char *sourceFile,
                          (afs_int32) estat.st_mode, estat.st_mtime);
 
     if (tst) {
-       rx_EndCall(tcall, tst);
+       tst = rx_EndCall(tcall, tst);
        goto fail_bos_ExecutableCreate;
     }
 
@@ -3184,7 +3184,7 @@ bos_LogGet(const void *serverHandle, const char *log,
   fail_bos_LogGet:
 
     if (have_call) {
-       rx_EndCall(tcall, 0);
+       tst = rx_EndCall(tcall, 0);
     }
 
     if (st != NULL) {

diff --git a/src/libadmin/vos/vsprocs.c b/src/libadmin/vos/vsprocs.c
index d5f6f6e..b54d995 100644 (file)
--- a/src/libadmin/vos/vsprocs.c
+++ b/src/libadmin/vos/vsprocs.c
@@ -2073,14 +2073,12 @@ UV_DumpVolume(afs_cell_handle_p cellHandle, afs_uint32 afromvol,
     struct rx_connection *fromconn;
     struct rx_call *fromcall;
     afs_int32 fromtid;
-    afs_int32 rxError;
     afs_int32 rcode;
 
     struct nvldbentry entry;
     int islocked;
 
     islocked = 0;
-    rxError = 0;
     fromconn = (struct rx_connection *)0;
     fromtid = 0;
     fromcall = (struct rx_call *)0;
@@ -2103,7 +2101,7 @@ UV_DumpVolume(afs_cell_handle_p cellHandle, afs_uint32 afromvol,
     if ((tst = DumpFunction(fromcall, filename))) {
        goto fail_UV_DumpVolume;
     }
-    tst = rx_EndCall(fromcall, rxError);
+    tst = rx_EndCall(fromcall, 0);
     fromcall = (struct rx_call *)0;
     if (tst) {
        goto fail_UV_DumpVolume;
@@ -2130,7 +2128,7 @@ UV_DumpVolume(afs_cell_handle_p cellHandle, afs_uint32 afromvol,
     }
 
     if (fromcall) {
-       etst = rx_EndCall(fromcall, rxError);
+       etst = rx_EndCall(fromcall, 0);
        if (etst) {
            if (!tst)
                tst = etst;
@@ -2265,7 +2263,6 @@ UV_RestoreVolume(afs_cell_handle_p cellHandle, afs_int32 toserver,
     struct rx_connection *toconn, *tempconn;
     struct rx_call *tocall;
     afs_int32 totid, rcode;
-    afs_int32 rxError = 0;
     struct volser_status tstatus;
     char partName[10];
     afs_uint32 pvolid;
@@ -2372,7 +2369,7 @@ UV_RestoreVolume(afs_cell_handle_p cellHandle, afs_int32 toserver,
     if (tst) {
        goto fail_UV_RestoreVolume;
     }
-    tst = rx_EndCall(tocall, rxError);
+    tst = rx_EndCall(tocall, 0);
     tocall = (struct rx_call *)0;
     if (tst) {
        goto fail_UV_RestoreVolume;
@@ -2512,7 +2509,7 @@ UV_RestoreVolume(afs_cell_handle_p cellHandle, afs_int32 toserver,
   fail_UV_RestoreVolume:
 
     if (tocall) {
-       etst = rx_EndCall(tocall, rxError);
+       etst = rx_EndCall(tocall, 0);
        if (!tst)
            tst = etst;
     }

diff --git a/src/libafscp/afscp_file.c b/src/libafscp/afscp_file.c
index b4bdc93..b887c5a 100644 (file)
--- a/src/libafscp/afscp_file.c
+++ b/src/libafscp/afscp_file.c
@@ -46,7 +46,7 @@ afscp_PRead(const struct afscp_venusfid * fid, void *buffer,
     struct afscp_volume *vol;
     struct afscp_server *server;
     struct rx_call *c = NULL;
-    int code, code2 = 0;
+    int code;
     int i, j, bytes, totalbytes = 0;
     int bytesremaining;
     char *p;
@@ -74,7 +74,7 @@ afscp_PRead(const struct afscp_venusfid * fid, void *buffer,
                        rx_Read(c, (char *)&bytesremaining,
                                sizeof(afs_int32));
                    if (bytes != sizeof(afs_int32)) {
-                       code = rx_EndCall(c, bytes);
+                       code = rx_EndCall(c, RX_PROTOCOL_ERROR);
                        continue;
                    }
                    bytesremaining = ntohl(bytesremaining);
@@ -89,12 +89,12 @@ afscp_PRead(const struct afscp_venusfid * fid, void *buffer,
                    }
                    if (bytesremaining == 0) {
                        time(&now);
-                       code2 = EndRXAFS_FetchData(c, &fst, &cb, &vs);
-                       if (code2 == 0)
+                       code = EndRXAFS_FetchData(c, &fst, &cb, &vs);
+                       if (code == 0)
                            afscp_AddCallBack(server, &fid->fid, &fst, &cb,
                                              now);
                    }
-                   code = rx_EndCall(c, code2);
+                   code = rx_EndCall(c, code);
                }
                if (code == 0) {
                    return totalbytes;
@@ -119,7 +119,7 @@ afscp_PWrite(const struct afscp_venusfid * fid, const void *buffer,
     struct afscp_volume *vol;
     struct afscp_server *server;
     struct rx_call *c = NULL;
-    int code, code2 = 0;
+    int code;
     int i, j, bytes, totalbytes = 0;
     int bytesremaining;
     const char *p;
@@ -184,9 +184,9 @@ afscp_PWrite(const struct afscp_venusfid * fid, const void *buffer,
                        bytesremaining -= bytes;
                    }
                    if (bytesremaining == 0) {
-                       code2 = EndRXAFS_StoreData(c, &fst, &vs);
+                       code = EndRXAFS_StoreData(c, &fst, &vs);
                    }
-                   code = rx_EndCall(c, code2);
+                   code = rx_EndCall(c, code);
                }
                if (code == 0) {
                    return totalbytes;

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 8501081..0b8cc48 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -2510,6 +2510,10 @@ rx_EndCall(struct rx_call *call, afs_int32 rc)
      * Map errors to the local host's errno.h format.
      */
     error = ntoh_syserr_conv(error);
+
+    /* If the caller said the call failed with some error, we had better
+     * return an error code. */
+    osi_Assert(!rc || error);
     return error;
 }
 

diff --git a/src/ubik/recovery.c b/src/ubik/recovery.c
index d355ab4..44c10fa 100644 (file)
--- a/src/ubik/recovery.c
+++ b/src/ubik/recovery.c
@@ -446,7 +446,7 @@ done:
 void *
 urecovery_Interact(void *dummy)
 {
-    afs_int32 code, tcode;
+    afs_int32 code;
     struct ubik_server *bestServer = NULL;
     struct ubik_server *ts;
     int dbok, doingRPC, now;
@@ -662,9 +662,7 @@ urecovery_Interact(void *dummy)
                goto FetchEndCall;
            code = EndDISK_GetFile(rxcall, &tversion);
          FetchEndCall:
-           tcode = rx_EndCall(rxcall, code);
-           if (!code)
-               code = tcode;
+           code = rx_EndCall(rxcall, code);
            if (!code) {
                /* we got a new file, set up its header */
                urecovery_state |= UBIK_RECHAVEDB;

diff --git a/src/volser/vsprocs.c b/src/volser/vsprocs.c
index 2083e8d..39d51fb 100644 (file)
--- a/src/volser/vsprocs.c
+++ b/src/volser/vsprocs.c
@@ -4542,7 +4542,7 @@ UV_DumpVolume(afs_uint32 afromvol, afs_uint32 afromserver, afs_int32 afrompart,
     struct rx_connection * volatile fromconn = NULL;
     afs_int32 volatile fromtid = 0;
 
-    afs_int32 rxError = 0, rcode = 0;
+    afs_int32 rcode = 0;
     afs_int32 code, error = 0;
     afs_int32 tmp;
     time_t tmv = fromdate;
@@ -4592,7 +4592,7 @@ UV_DumpVolume(afs_uint32 afromvol, afs_uint32 afromserver, afs_int32 afrompart,
 
   error_exit:
     if (fromcall) {
-       code = rx_EndCall(fromcall, rxError);
+       code = rx_EndCall(fromcall, 0);
        if (code && code != RXGEN_OPCODE)
            fprintf(STDERR, "Error in rx_EndCall\n");
        if (code && !error)
@@ -4637,7 +4637,7 @@ UV_DumpClonedVolume(afs_uint32 afromvol, afs_uint32 afromserver,
     afs_uint32 volatile clonevol = 0;
 
     afs_int32 tmp;
-    afs_int32 fromtid = 0, rxError = 0, rcode = 0;
+    afs_int32 fromtid = 0, rcode = 0;
     afs_int32 code = 0, error = 0;
     afs_uint32 tmpVol;
     char vname[64];
@@ -4745,7 +4745,7 @@ UV_DumpClonedVolume(afs_uint32 afromvol, afs_uint32 afromserver,
     }
 
     if (fromcall) {
-       code = rx_EndCall(fromcall, rxError);
+       code = rx_EndCall(fromcall, 0);
        if (code) {
            fprintf(STDERR, "Error in rx_EndCall\n");
            if (!error)
@@ -4787,7 +4787,6 @@ UV_RestoreVolume2(afs_uint32 toserver, afs_int32 topart, afs_uint32 tovolid,
     struct rx_connection *toconn, *tempconn;
     struct rx_call *tocall;
     afs_int32 totid, code, rcode, vcode, terror = 0;
-    afs_int32 rxError = 0;
     struct volser_status tstatus;
     struct volintInfo vinfo;
     char partName[10];
@@ -4933,7 +4932,7 @@ UV_RestoreVolume2(afs_uint32 toserver, afs_int32 topart, afs_uint32 tovolid,
        error = code;
        goto refail;
     }
-    terror = rx_EndCall(tocall, rxError);
+    terror = rx_EndCall(tocall, 0);
     tocall = (struct rx_call *)0;
     if (terror) {
        fprintf(STDERR, "rx_EndCall Failed \n");
@@ -5157,7 +5156,7 @@ UV_RestoreVolume2(afs_uint32 toserver, afs_int32 topart, afs_uint32 tovolid,
     }
   refail:
     if (tocall) {
-       code = rx_EndCall(tocall, rxError);
+       code = rx_EndCall(tocall, 0);
        if (!error)
            error = code;
     }