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 21f52b5af19484ba5b083c6b6d3a3c7d7fb3093b..6da6153ba18591329f0cc1f87f4c7585e9e4df6b 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 5b9d3a2786fcf6b04e3e981e05a523cc45dead06..491ef44a5664b17e814f098cff10939c9915a4e0 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 2e36d00658468ecdc5aa3b8eb913b4c0b339b884..508ccfcff86b333548fbbe04349710a332d22c8d 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 99f9c5153e26126711d1cc96fedaba669e306862..3dc865bea7ed9a6a291eca01905ef04c1aade38e 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 3edae25543058ed86c04cc969fde8a306daa4775..816853f5c95abd2155ba5459c5a01e5546b52a67 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 0d7cb388ec250c187c511c028dff368dd354ef95..da379197b0281c8321fea52a91b3ce5d5d751e9b 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 d5f6f6e62a46d304ee8989bacc591180bdae5c48..b54d9956a8f4aa1446b7aa90a07261694fb1d745 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 b4bdc93f39f02f8f73a071b9787cb8d860f37858..b887c5ac470a901616981ff96c6da5737c2267bc 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 850108195c9d24decb6085cdf536715fa7f0126d..0b8cc48bac5bfd3cac068d8edf299de364c3f71d 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 d355ab4bdc78f2742f128933cca4509b9a55459a..44c10faf54c55c4411b3779cd24a78073cb991fe 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 2083e8df40207894b1a39c38c901ff119d0dcab7..39d51fbd6f5bc9ce5199fba38f69643ae3233ddc 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;
     }