PuTTy: Diff

Differences From Artifact [a04afbb955]:

File sftp.c — part of check-in [14608d0542] at 2013-07-06 15:43:21 on branch trunk — Clean up handling of the return value from sftp_find_request. In many places we simply enforce by assertion that it will match the request we sent out a moment ago: in fact it can also return NULL, so it makes more sense to report a proper error message if it doesn't return the expected value, and while we're at it, have that error message whatever message was helpfully left in fxp_error() by sftp_find_request when it failed. To do this, I've written a centralised function in psftp.c called sftp_wait_for_reply, which is handed a request that's just been sent out and deals with the mechanics of waiting for its reply, returning the reply when it arrives, and aborting with a sensible error if anything else arrives instead. The numerous sites in psftp.c which called sftp_find_request have all been rewritten to do this instead, and as a side effect they now look more sensible. The only other uses of sftp_find_request were in xfer_*load_gotpkt, which had to be tweaked in its own way. While I'm here, also fix memory management in sftp_find_request, which was freeing its input packet on some but not all error return paths. (user: simon size: 34526)

To Artifact [01175426be]:

File sftp.c — part of check-in [1129bffc22] at 2013-07-11 12:24:53 on branch trunk — xfer_{up,down}load_gotpkt free their input sftp_packet as a side effect of handling it, but they do not free it if it isn't a packet they recognise as part of their upload/download. Invent a return value that specifically signals this, and consistently free pktin at every call site if that return value comes back. Also, ensure that that return value also always comes with something meaningful in fxp_error. (user: simon size: 34961)

︙
1192 1193 1194 1195 1196 1197 1198 1199 1200 1201 1202 1203 1204 1205 ~~1206~~ 1207 ~~1208 1209~~ 1210 1211 1212 1213 1214 1215 1216	1192 1193 1194 1195 1196 1197 1198 1199 1200 1201 1202 1203 1204 1205 1206 1207 1208 1209 1210 1211 1212 1213 1214 1215 1216 1217 1218 1219 1220 1221 1222	+ + + + - + - - + + + +	xfer->eof = FALSE; xfer_download_queue(xfer); return xfer; } /* * Returns INT_MIN to indicate that it didn't even get as far as * fxp_read_recv and hence has not freed pktin. / int xfer_download_gotpkt(struct fxp_xfer xfer, struct sftp_packet pktin) { struct sftp_request rreq; struct req rr; rreq = sftp_find_request(pktin); if (!rreq) ~~return 0; / this packet doesn't even make sense /~~ return INT_MIN; / this packet doesn't even make sense / rr = (struct req )fxp_get_userdata(rreq); ~~if (!rr) return 0; /* this packet isn't ours /~~ if (!rr) { fxp_internal_error("request ID is not part of the current download"); return INT_MIN; / this packet isn't ours */ } rr->retlen = fxp_read_recv(pktin, rreq, rr->buffer, rr->len); #ifdef DEBUG_DOWNLOAD printf("read request %p has returned [%d]\n", rr, rr->retlen); #endif if ((rr->retlen < 0 && fxp_error_type()==SSH_FX_EOF) \|\| rr->retlen == 0) { xfer->eof = TRUE;
︙
1373 1374 1375 1376 1377 1378 1379 1380 1381 1382 1383 1384 1385 1386 1387 ~~1388~~ 1389 ~~1390 1391~~ 1392 1393 1394 1395 1396 1397 1398	1379 1380 1381 1382 1383 1384 1385 1386 1387 1388 1389 1390 1391 1392 1393 1394 1395 1396 1397 1398 1399 1400 1401 1402 1403 1404 1405 1406 1407 1408 1409 1410	+ + + + - + - - + + + +	xfer->req_totalsize += rr->len; #ifdef DEBUG_UPLOAD { char buf[40]; uint64_decimal(rr->offset, buf); printf("queueing write request %p at %s [len %d]\n", rr, buf, len); } #endif } /* * Returns INT_MIN to indicate that it didn't even get as far as * fxp_write_recv and hence has not freed pktin. / int xfer_upload_gotpkt(struct fxp_xfer xfer, struct sftp_packet pktin) { struct sftp_request rreq; struct req rr, prev, next; int ret; rreq = sftp_find_request(pktin); if (!rreq) ~~return 0; / this packet doesn't even make sense /~~ return INT_MIN; / this packet doesn't even make sense / rr = (struct req )fxp_get_userdata(rreq); ~~if (!rr) return 0; /* this packet isn't ours /~~ if (!rr) { fxp_internal_error("request ID is not part of the current upload"); return INT_MIN; / this packet isn't ours / } ret = fxp_write_recv(pktin, rreq); #ifdef DEBUG_UPLOAD printf("write request %p has returned [%d]\n", rr, ret); #endif / * Remove this one from the queue.
︙