From 8d26e7ba3239088173363c4b1f2c8ff084ec03c2 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 17 Sep 2020 15:15:08 +0200 Subject: [PATCH 1/3] ftp: get rid of the PPSENDF macro The use of such a macro hides some of what's actually going on to the reader and is generally disapproved of in the project. --- lib/ftp.c | 292 ++++++++++++++++++++++++++---------------------------- 1 file changed, 143 insertions(+), 149 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index d4ecf9a9608347..f1070592def1e2 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -137,14 +137,10 @@ static int ftp_domore_getsock(struct connectdata *conn, curl_socket_t *socks); static CURLcode ftp_doing(struct connectdata *conn, bool *dophase_done); static CURLcode ftp_setup_connection(struct connectdata *conn); - static CURLcode init_wc_data(struct connectdata *conn); static CURLcode wc_statemach(struct connectdata *conn); - static void wc_data_dtor(void *ptr); - static CURLcode ftp_state_retr(struct connectdata *conn, curl_off_t filesize); - static CURLcode ftp_readresp(curl_socket_t sockfd, struct pingpong *pp, int *ftpcode, @@ -152,12 +148,6 @@ static CURLcode ftp_readresp(curl_socket_t sockfd, static CURLcode ftp_dophase_done(struct connectdata *conn, bool connected); -/* easy-to-use macro: */ -#define PPSENDF(x,y,z) result = Curl_pp_sendf(x,y,z); \ - if(result) \ - return result - - /* * FTP protocol handler. */ @@ -775,25 +765,22 @@ static void _state(struct connectdata *conn, static CURLcode ftp_state_user(struct connectdata *conn) { - CURLcode result; - /* send USER */ - PPSENDF(&conn->proto.ftpc.pp, "USER %s", conn->user?conn->user:""); - - state(conn, FTP_USER); - conn->data->state.ftp_trying_alternative = FALSE; - - return CURLE_OK; + CURLcode result = Curl_pp_sendf(&conn->proto.ftpc.pp, "USER %s", + conn->user?conn->user:""); + if(!result) { + state(conn, FTP_USER); + conn->data->state.ftp_trying_alternative = FALSE; + } + return result; } static CURLcode ftp_state_pwd(struct connectdata *conn) { - CURLcode result; - - /* send PWD to discover our entry point */ - PPSENDF(&conn->proto.ftpc.pp, "%s", "PWD"); - state(conn, FTP_PWD); + CURLcode result = Curl_pp_sendf(&conn->proto.ftpc.pp, "%s", "PWD"); + if(!result) + state(conn, FTP_PWD); - return CURLE_OK; + return result; } /* For the FTP "protocol connect" and "doing" phases only */ @@ -881,16 +868,19 @@ static CURLcode ftp_state_cwd(struct connectdata *conn) where we ended up after login: */ ftpc->cwdcount = 0; /* we count this as the first path, then we add one for all upcoming ones in the ftp->dirs[] array */ - PPSENDF(&conn->proto.ftpc.pp, "CWD %s", ftpc->entrypath); - state(conn, FTP_CWD); + result = Curl_pp_sendf(&ftpc->pp, "CWD %s", ftpc->entrypath); + if(!result) + state(conn, FTP_CWD); } else { if(ftpc->dirdepth) { ftpc->cwdcount = 1; /* issue the first CWD, the rest is sent when the CWD responses are received... */ - PPSENDF(&conn->proto.ftpc.pp, "CWD %s", ftpc->dirs[ftpc->cwdcount -1]); - state(conn, FTP_CWD); + result = Curl_pp_sendf(&ftpc->pp, "CWD %s", + ftpc->dirs[ftpc->cwdcount -1]); + if(!result) + state(conn, FTP_CWD); } else { /* No CWD necessary */ @@ -1326,12 +1316,12 @@ static CURLcode ftp_state_use_pasv(struct connectdata *conn) modeoff = conn->bits.ftp_use_epsv?0:1; - PPSENDF(&ftpc->pp, "%s", mode[modeoff]); - - ftpc->count1 = modeoff; - state(conn, FTP_PASV); - infof(conn->data, "Connect data stream passively\n"); - + result = Curl_pp_sendf(&ftpc->pp, "%s", mode[modeoff]); + if(!result) { + ftpc->count1 = modeoff; + state(conn, FTP_PASV); + infof(conn->data, "Connect data stream passively\n"); + } return result; } @@ -1364,23 +1354,23 @@ static CURLcode ftp_state_prepare_transfer(struct connectdata *conn) if(data->set.ftp_use_pret) { /* The user has requested that we send a PRET command to prepare the server for the upcoming PASV */ - if(!conn->proto.ftpc.file) { - PPSENDF(&conn->proto.ftpc.pp, "PRET %s", - data->set.str[STRING_CUSTOMREQUEST]? - data->set.str[STRING_CUSTOMREQUEST]: - (data->set.ftp_list_only?"NLST":"LIST")); - } - else if(data->set.upload) { - PPSENDF(&conn->proto.ftpc.pp, "PRET STOR %s", conn->proto.ftpc.file); - } - else { - PPSENDF(&conn->proto.ftpc.pp, "PRET RETR %s", conn->proto.ftpc.file); - } - state(conn, FTP_PRET); + struct ftp_conn *ftpc = &conn->proto.ftpc; + if(!conn->proto.ftpc.file) + result = Curl_pp_sendf(&ftpc->pp, "PRET %s", + data->set.str[STRING_CUSTOMREQUEST]? + data->set.str[STRING_CUSTOMREQUEST]: + (data->set.ftp_list_only?"NLST":"LIST")); + else if(data->set.upload) + result = Curl_pp_sendf(&ftpc->pp, "PRET STOR %s", + conn->proto.ftpc.file); + else + result = Curl_pp_sendf(&ftpc->pp, "PRET RETR %s", + conn->proto.ftpc.file); + if(!result) + state(conn, FTP_PRET); } - else { + else result = ftp_state_use_pasv(conn); - } } return result; } @@ -1396,9 +1386,9 @@ static CURLcode ftp_state_rest(struct connectdata *conn) /* Determine if server can respond to REST command and therefore whether it supports range */ - PPSENDF(&conn->proto.ftpc.pp, "REST %d", 0); - - state(conn, FTP_REST); + result = Curl_pp_sendf(&ftpc->pp, "REST %d", 0); + if(!result) + state(conn, FTP_REST); } else result = ftp_state_prepare_transfer(conn); @@ -1416,9 +1406,9 @@ static CURLcode ftp_state_size(struct connectdata *conn) /* if a "head"-like request is being made (on a file) */ /* we know ftpc->file is a valid pointer to a file name */ - PPSENDF(&ftpc->pp, "SIZE %s", ftpc->file); - - state(conn, FTP_SIZE); + result = Curl_pp_sendf(&ftpc->pp, "SIZE %s", ftpc->file); + if(!result) + state(conn, FTP_SIZE); } else result = ftp_state_rest(conn); @@ -1485,10 +1475,8 @@ static CURLcode ftp_state_list(struct connectdata *conn) result = Curl_pp_sendf(&conn->proto.ftpc.pp, "%s", cmd); free(cmd); - if(result) - return result; - - state(conn, FTP_LIST); + if(!result) + state(conn, FTP_LIST); return result; } @@ -1549,9 +1537,10 @@ static CURLcode ftp_state_mdtm(struct connectdata *conn) /* we have requested to get the modified-time of the file, this is a white spot as the MDTM is not mentioned in RFC959 */ - PPSENDF(&ftpc->pp, "MDTM %s", ftpc->file); + result = Curl_pp_sendf(&ftpc->pp, "MDTM %s", ftpc->file); - state(conn, FTP_MDTM); + if(!result) + state(conn, FTP_MDTM); } else result = ftp_state_type(conn); @@ -1587,8 +1576,9 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn, if(data->state.resume_from < 0) { /* Got no given size to start from, figure it out */ - PPSENDF(&ftpc->pp, "SIZE %s", ftpc->file); - state(conn, FTP_STOR_SIZE); + result = Curl_pp_sendf(&ftpc->pp, "SIZE %s", ftpc->file); + if(!result) + state(conn, FTP_STOR_SIZE); return result; } @@ -1650,10 +1640,10 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn, /* we've passed, proceed as normal */ } /* resume_from */ - PPSENDF(&ftpc->pp, data->set.ftp_append?"APPE %s":"STOR %s", - ftpc->file); - - state(conn, FTP_STOR); + result = Curl_pp_sendf(&ftpc->pp, data->set.ftp_append?"APPE %s":"STOR %s", + ftpc->file); + if(!result) + state(conn, FTP_STOR); return result; } @@ -1711,7 +1701,9 @@ static CURLcode ftp_state_quote(struct connectdata *conn, else ftpc->count2 = 0; /* failure means cancel operation */ - PPSENDF(&ftpc->pp, "%s", cmd); + result = Curl_pp_sendf(&ftpc->pp, "%s", cmd); + if(result) + return result; state(conn, instate); quote = TRUE; } @@ -1740,12 +1732,14 @@ static CURLcode ftp_state_quote(struct connectdata *conn, the server terminates it, otherwise the client stops if the received byte count exceeds the reported file size. Set option CURLOPT_IGNORE_CONTENT_LENGTH to 1 to enable this behavior.*/ - PPSENDF(&ftpc->pp, "RETR %s", ftpc->file); - state(conn, FTP_RETR); + result = Curl_pp_sendf(&ftpc->pp, "RETR %s", ftpc->file); + if(!result) + state(conn, FTP_RETR); } else { - PPSENDF(&ftpc->pp, "SIZE %s", ftpc->file); - state(conn, FTP_RETR_SIZE); + result = Curl_pp_sendf(&ftpc->pp, "SIZE %s", ftpc->file); + if(!result) + state(conn, FTP_RETR_SIZE); } } } @@ -1782,10 +1776,12 @@ static CURLcode ftp_epsv_disable(struct connectdata *conn) conn->bits.ftp_use_epsv = FALSE; conn->data->state.errorbuf = FALSE; /* allow error message to get rewritten */ - PPSENDF(&conn->proto.ftpc.pp, "%s", "PASV"); - conn->proto.ftpc.count1++; - /* remain in/go to the FTP_PASV state */ - state(conn, FTP_PASV); + result = Curl_pp_sendf(&conn->proto.ftpc.pp, "%s", "PASV"); + if(!result) { + conn->proto.ftpc.count1++; + /* remain in/go to the FTP_PASV state */ + state(conn, FTP_PASV); + } return result; } @@ -2229,15 +2225,16 @@ static CURLcode ftp_state_retr(struct connectdata *conn, infof(data, "Instructs server to resume from offset %" CURL_FORMAT_CURL_OFF_T "\n", data->state.resume_from); - PPSENDF(&ftpc->pp, "REST %" CURL_FORMAT_CURL_OFF_T, - data->state.resume_from); - - state(conn, FTP_RETR_REST); + result = Curl_pp_sendf(&ftpc->pp, "REST %" CURL_FORMAT_CURL_OFF_T, + data->state.resume_from); + if(!result) + state(conn, FTP_RETR_REST); } else { /* no resume */ - PPSENDF(&ftpc->pp, "RETR %s", ftpc->file); - state(conn, FTP_RETR); + result = Curl_pp_sendf(&ftpc->pp, "RETR %s", ftpc->file); + if(!result) + state(conn, FTP_RETR); } return result; @@ -2330,8 +2327,9 @@ static CURLcode ftp_state_rest_resp(struct connectdata *conn, result = CURLE_FTP_COULDNT_USE_REST; } else { - PPSENDF(&ftpc->pp, "RETR %s", ftpc->file); - state(conn, FTP_RETR); + result = Curl_pp_sendf(&ftpc->pp, "RETR %s", ftpc->file); + if(!result) + state(conn, FTP_RETR); } break; } @@ -2523,8 +2521,9 @@ static CURLcode ftp_state_loggedin(struct connectdata *conn) parameter of '0' to indicate that no buffering is taking place and the data connection should not be encapsulated. */ - PPSENDF(&conn->proto.ftpc.pp, "PBSZ %d", 0); - state(conn, FTP_PBSZ); + result = Curl_pp_sendf(&conn->proto.ftpc.pp, "PBSZ %d", 0); + if(!result) + state(conn, FTP_PBSZ); } else { result = ftp_state_pwd(conn); @@ -2546,8 +2545,9 @@ static CURLcode ftp_state_user_resp(struct connectdata *conn, if((ftpcode == 331) && (ftpc->state == FTP_USER)) { /* 331 Password required for ... (the server requires to send the user's password too) */ - PPSENDF(&ftpc->pp, "PASS %s", conn->passwd?conn->passwd:""); - state(conn, FTP_PASS); + result = Curl_pp_sendf(&ftpc->pp, "PASS %s", conn->passwd?conn->passwd:""); + if(!result) + state(conn, FTP_PASS); } else if(ftpcode/100 == 2) { /* 230 User ... logged in. @@ -2556,8 +2556,10 @@ static CURLcode ftp_state_user_resp(struct connectdata *conn, } else if(ftpcode == 332) { if(data->set.str[STRING_FTP_ACCOUNT]) { - PPSENDF(&ftpc->pp, "ACCT %s", data->set.str[STRING_FTP_ACCOUNT]); - state(conn, FTP_ACCT); + result = Curl_pp_sendf(&ftpc->pp, "ACCT %s", + data->set.str[STRING_FTP_ACCOUNT]); + if(!result) + state(conn, FTP_ACCT); } else { failf(data, "ACCT requested but none available"); @@ -2573,11 +2575,13 @@ static CURLcode ftp_state_user_resp(struct connectdata *conn, if(conn->data->set.str[STRING_FTP_ALTERNATIVE_TO_USER] && !conn->data->state.ftp_trying_alternative) { /* Ok, USER failed. Let's try the supplied command. */ - PPSENDF(&conn->proto.ftpc.pp, "%s", - conn->data->set.str[STRING_FTP_ALTERNATIVE_TO_USER]); - conn->data->state.ftp_trying_alternative = TRUE; - state(conn, FTP_USER); - result = CURLE_OK; + result = + Curl_pp_sendf(&ftpc->pp, "%s", + conn->data->set.str[STRING_FTP_ALTERNATIVE_TO_USER]); + if(!result) { + conn->data->state.ftp_trying_alternative = TRUE; + state(conn, FTP_USER); + } } else { failf(data, "Access denied: %03d", ftpcode); @@ -2679,15 +2683,12 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) (int)data->set.ftpsslauth); return CURLE_UNKNOWN_OPTION; /* we don't know what to do */ } - PPSENDF(&ftpc->pp, "AUTH %s", ftpauth[ftpc->count1]); - state(conn, FTP_AUTH); + result = Curl_pp_sendf(&ftpc->pp, "AUTH %s", ftpauth[ftpc->count1]); + if(!result) + state(conn, FTP_AUTH); } - else { + else result = ftp_state_user(conn); - if(result) - return result; - } - break; case FTP_AUTH: @@ -2722,9 +2723,6 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) /* ignore the failure and continue */ result = ftp_state_user(conn); } - - if(result) - return result; break; case FTP_USER: @@ -2737,10 +2735,11 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) break; case FTP_PBSZ: - PPSENDF(&ftpc->pp, "PROT %c", - data->set.use_ssl == CURLUSESSL_CONTROL ? 'C' : 'P'); - state(conn, FTP_PROT); - + result = + Curl_pp_sendf(&ftpc->pp, "PROT %c", + data->set.use_ssl == CURLUSESSL_CONTROL ? 'C' : 'P'); + if(!result) + state(conn, FTP_PROT); break; case FTP_PROT: @@ -2757,14 +2756,12 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) if(data->set.ftp_ccc) { /* CCC - Clear Command Channel */ - PPSENDF(&ftpc->pp, "%s", "CCC"); - state(conn, FTP_CCC); + result = Curl_pp_sendf(&ftpc->pp, "%s", "CCC"); + if(!result) + state(conn, FTP_CCC); } - else { + else result = ftp_state_pwd(conn); - if(result) - return result; - } break; case FTP_CCC: @@ -2772,16 +2769,12 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) /* First shut down the SSL layer (note: this call will block) */ result = Curl_ssl_shutdown(conn, FIRSTSOCKET); - if(result) { + if(result) failf(conn->data, "Failed to clear the command channel (CCC)"); - return result; - } } - - /* Then continue as normal */ - result = ftp_state_pwd(conn); - if(result) - return result; + if(!result) + /* Then continue as normal */ + result = ftp_state_pwd(conn); break; case FTP_PWD: @@ -2847,7 +2840,6 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) systems. */ if(!ftpc->server_os && dir[0] != '/') { - result = Curl_pp_sendf(&ftpc->pp, "%s", "SYST"); if(result) { free(dir); @@ -2943,12 +2935,10 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) if((ftpcode >= 400) && !ftpc->count2) { /* failure response code, and not allowed to fail */ failf(conn->data, "QUOT command failed with %03d", ftpcode); - return CURLE_QUOTE_ERROR; + result = CURLE_QUOTE_ERROR; } - result = ftp_state_quote(conn, FALSE, ftpc->state); - if(result) - return result; - + else + result = ftp_state_quote(conn, FALSE, ftpc->state); break; case FTP_CWD: @@ -2958,29 +2948,28 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) ftpc->cwdcount && !ftpc->count2) { /* try making it */ ftpc->count2++; /* counter to prevent CWD-MKD loops */ - PPSENDF(&ftpc->pp, "MKD %s", ftpc->dirs[ftpc->cwdcount - 1]); - state(conn, FTP_MKD); + result = Curl_pp_sendf(&ftpc->pp, "MKD %s", + ftpc->dirs[ftpc->cwdcount - 1]); + if(!result) + state(conn, FTP_MKD); } else { /* return failure */ failf(data, "Server denied you to change to the given directory"); ftpc->cwdfail = TRUE; /* don't remember this path as we failed to enter it */ - return CURLE_REMOTE_ACCESS_DENIED; + result = CURLE_REMOTE_ACCESS_DENIED; } } else { /* success */ ftpc->count2 = 0; - if(++ftpc->cwdcount <= ftpc->dirdepth) { + if(++ftpc->cwdcount <= ftpc->dirdepth) /* send next CWD */ - PPSENDF(&ftpc->pp, "CWD %s", ftpc->dirs[ftpc->cwdcount - 1]); - } - else { + result = Curl_pp_sendf(&ftpc->pp, "CWD %s", + ftpc->dirs[ftpc->cwdcount - 1]); + else result = ftp_state_mdtm(conn); - if(result) - return result; - } } break; @@ -2988,11 +2977,14 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) if((ftpcode/100 != 2) && !ftpc->count3--) { /* failure to MKD the dir */ failf(data, "Failed to MKD dir: %03d", ftpcode); - return CURLE_REMOTE_ACCESS_DENIED; + result = CURLE_REMOTE_ACCESS_DENIED; + } + else { + state(conn, FTP_CWD); + /* send CWD */ + result = Curl_pp_sendf(&ftpc->pp, "CWD %s", + ftpc->dirs[ftpc->cwdcount - 1]); } - state(conn, FTP_CWD); - /* send CWD */ - PPSENDF(&ftpc->pp, "CWD %s", ftpc->dirs[ftpc->cwdcount - 1]); break; case FTP_MDTM: @@ -3394,7 +3386,7 @@ CURLcode ftp_sendquote(struct connectdata *conn, struct curl_slist *quote) acceptfail = TRUE; } - PPSENDF(&conn->proto.ftpc.pp, "%s", cmd); + result = Curl_pp_sendf(&ftpc->pp, "%s", cmd); pp->response = Curl_now(); /* timeout relative now */ @@ -3446,12 +3438,14 @@ static CURLcode ftp_nb_type(struct connectdata *conn, return ftp_state_type_resp(conn, 200, newstate); } - PPSENDF(&ftpc->pp, "TYPE %c", want); - state(conn, newstate); + result = Curl_pp_sendf(&ftpc->pp, "TYPE %c", want); + if(!result) { + state(conn, newstate); - /* keep track of our current transfer type */ - ftpc->transfertype = want; - return CURLE_OK; + /* keep track of our current transfer type */ + ftpc->transfertype = want; + } + return result; } /*************************************************************************** From 718370e9f1c8b0aaf026ca0701db655bde06cb50 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 17 Sep 2020 15:38:35 +0200 Subject: [PATCH 2/3] fixup DEAD_STORE: The value written to result (type int) is never used --- lib/ftp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index f1070592def1e2..3e04ff26c4d387 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -3366,7 +3366,6 @@ CURLcode ftp_sendquote(struct connectdata *conn, struct curl_slist *quote) struct curl_slist *item; ssize_t nread; int ftpcode; - CURLcode result; struct ftp_conn *ftpc = &conn->proto.ftpc; struct pingpong *pp = &ftpc->pp; @@ -3375,6 +3374,7 @@ CURLcode ftp_sendquote(struct connectdata *conn, struct curl_slist *quote) if(item->data) { char *cmd = item->data; bool acceptfail = FALSE; + CURLcode result; /* if a command starts with an asterisk, which a legal FTP command never can, the command will be allowed to fail without it causing any @@ -3387,10 +3387,10 @@ CURLcode ftp_sendquote(struct connectdata *conn, struct curl_slist *quote) } result = Curl_pp_sendf(&ftpc->pp, "%s", cmd); - - pp->response = Curl_now(); /* timeout relative now */ - - result = Curl_GetFTPResponse(&nread, conn, &ftpcode); + if(!result) { + pp->response = Curl_now(); /* timeout relative now */ + result = Curl_GetFTPResponse(&nread, conn, &ftpcode); + } if(result) return result; From d27b24641a4efb45686da744231979d7fc4fb0d4 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 17 Sep 2020 16:00:48 +0200 Subject: [PATCH 3/3] fixup please code analyzer and init ftpcode --- lib/ftp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index 3e04ff26c4d387..868a97a532fd31 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -3364,17 +3364,17 @@ static CURLcode ftp_sendquote(struct connectdata *conn, struct curl_slist *quote) { struct curl_slist *item; - ssize_t nread; - int ftpcode; struct ftp_conn *ftpc = &conn->proto.ftpc; struct pingpong *pp = &ftpc->pp; item = quote; while(item) { if(item->data) { + ssize_t nread; char *cmd = item->data; bool acceptfail = FALSE; CURLcode result; + int ftpcode = 0; /* if a command starts with an asterisk, which a legal FTP command never can, the command will be allowed to fail without it causing any