From a10696d9408b0ac21f8a011fce0a6a72b1c0fe0e Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Thu, 30 Jan 2014 13:50:11 -0600 Subject: [PATCH] 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 Tested-by: BuildBot --- src/WINNT/afsd/cm_dcache.c | 62 +++++++++++++++++---------------- src/WINNT/afsd/cm_direct.c | 10 ++---- src/afs/afs_bypasscache.c | 3 +- src/afs/afs_fetchstore.c | 37 ++++++-------------- src/bozo/bos.c | 4 +-- src/libadmin/bos/afs_bosAdmin.c | 4 +-- src/libadmin/vos/vsprocs.c | 11 +++--- src/libafscp/afscp_file.c | 16 ++++----- src/rx/rx.c | 4 +++ src/ubik/recovery.c | 6 ++-- src/volser/vsprocs.c | 13 ++++--- 11 files changed, 75 insertions(+), 95 deletions(-) diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c index 21f52b5af..6da6153ba 100644 --- 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 5b9d3a278..491ef44a5 100644 --- 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 2e36d0065..508ccfcff 100644 --- 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 99f9c5153..3dc865bea 100644 --- 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 3edae2554..816853f5c 100644 --- 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 0d7cb388e..da379197b 100644 --- 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 d5f6f6e62..b54d9956a 100644 --- 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 b4bdc93f3..b887c5ac4 100644 --- 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 850108195..0b8cc48ba 100644 --- 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 d355ab4bd..44c10faf5 100644 --- 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 2083e8df4..39d51fbd6 100644 --- 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; } -- 2.39.5