From c925526e3fbf580a4df3e796fc78e22bbe33651d Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 11 Jul 2013 17:24:53 +0000 Subject: [PATCH] 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. [originally from svn r9915] --- pscp.c | 8 ++++++++ psftp.c | 12 +++++++++--- sftp.c | 24 ++++++++++++++++++------ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/pscp.c b/pscp.c index 5aedfee0..50bea930 100644 --- a/pscp.c +++ b/pscp.c @@ -928,6 +928,8 @@ int scp_send_filedata(char *data, int len) ret = xfer_upload_gotpkt(scp_sftp_xfer, pktin); if (ret <= 0) { tell_user(stderr, "error while writing: %s", fxp_error()); + if (ret == INT_MIN) /* pktin not even freed */ + sfree(pktin); errs++; return 1; } @@ -969,6 +971,8 @@ int scp_send_finish(void) ret = xfer_upload_gotpkt(scp_sftp_xfer, pktin); if (ret <= 0) { tell_user(stderr, "error while writing: %s", fxp_error()); + if (ret == INT_MIN) /* pktin not even freed */ + sfree(pktin); errs++; return 1; } @@ -1566,6 +1570,8 @@ int scp_recv_filedata(char *data, int len) ret = xfer_download_gotpkt(scp_sftp_xfer, pktin); if (ret <= 0) { tell_user(stderr, "pscp: error while reading: %s", fxp_error()); + if (ret == INT_MIN) /* pktin not even freed */ + sfree(pktin); errs++; return -1; } @@ -1611,6 +1617,8 @@ int scp_finish_filerecv(void) ret = xfer_download_gotpkt(scp_sftp_xfer, pktin); if (ret <= 0) { tell_user(stderr, "pscp: error while reading: %s", fxp_error()); + if (ret == INT_MIN) /* pktin not even freed */ + sfree(pktin); errs++; return -1; } diff --git a/psftp.c b/psftp.c index 26f24f9c..5bc7d7c8 100644 --- a/psftp.c +++ b/psftp.c @@ -468,6 +468,8 @@ int sftp_get_file(char *fname, char *outfname, int recurse, int restart) printf("error while reading: %s\n", fxp_error()); shown_err = TRUE; } + if (ret == INT_MIN) /* pktin not even freed */ + sfree(pktin); ret = 0; } @@ -720,9 +722,13 @@ int sftp_put_file(char *fname, char *outfname, int recurse, int restart) if (!xfer_done(xfer)) { pktin = sftp_recv(); ret = xfer_upload_gotpkt(xfer, pktin); - if (ret <= 0 && !err) { - printf("error while writing: %s\n", fxp_error()); - err = 1; + if (ret <= 0) { + if (ret == INT_MIN) /* pktin not even freed */ + sfree(pktin); + if (!err) { + printf("error while writing: %s\n", fxp_error()); + err = 1; + } } } } diff --git a/sftp.c b/sftp.c index 4c051826..927f4d9c 100644 --- a/sftp.c +++ b/sftp.c @@ -1196,6 +1196,10 @@ struct fxp_xfer *xfer_download_init(struct fxp_handle *fh, uint64 offset) 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; @@ -1203,10 +1207,12 @@ int xfer_download_gotpkt(struct fxp_xfer *xfer, struct sftp_packet *pktin) 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); @@ -1377,6 +1383,10 @@ void xfer_upload_data(struct fxp_xfer *xfer, char *buffer, int 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; @@ -1385,10 +1395,12 @@ int xfer_upload_gotpkt(struct fxp_xfer *xfer, struct sftp_packet *pktin) 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); -- 2.45.2