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]

diff --git a/pscp.c b/pscp.c
index 5aedfee0704a6d35ac434a67a82dd8b5f4c5d6ac..50bea9300bcbffac40b5d10eb9ef094638c20a9c 100644 (file)
--- 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 26f24f9ce7f04da388e53cdd2ef0e3950c40311d..5bc7d7c891e6212f8ab4bcdecb419b0d83441840 100644 (file)
--- 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 4c0518262e0e444dcb0901e088572dd3560170c3..927f4d9c4bd0c62f0cf41f0fce4b8a8b57b14406 100644 (file)
--- 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);