Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simple fix for github issue #3 #4

Closed
wants to merge 2 commits into from
Closed

Simple fix for github issue #3 #4

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
450e2e0
Simple fix for github issue #3
msalle May 20, 2020
207de8d
Just check for return code part of errmsg
msalle May 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cmds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3530,6 +3530,12 @@ _c_xfer_file(ch_t * sch,
if (ecl || ecr || ec)
cr = CMD_ERR_GET;

if (ec && strncmp(ec_get_errmsg(ec), "550 File exists", 15) == 0 ||
ecr && strncmp(ec_get_errmsg(ecr),"550 File exists", 15) == 0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: should be more robust to compare against just the number. According to https://tools.ietf.org/html/rfc959 4.2, the reply is defined to be a 3-digit code followed by a space, but the text afterward can be anything.

Suggested change
if (ec && strncmp(ec_get_errmsg(ec), "550 File exists", 15) == 0 ||
ecr && strncmp(ec_get_errmsg(ecr),"550 File exists", 15) == 0)
/* file exists */
if (ec && strncmp(ec_get_errmsg(ec), "550 ", 4) == 0 ||
ecr && strncmp(ec_get_errmsg(ecr),"550 ", 4) == 0)

Copy link
Member Author

@msalle msalle May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had considered that, then thought not to since 550 is a bit more generic (there can be different reasons than that the file already exists in which can we might need to delete it after all), but I think you're right it's probably still the most reliable check now. I now check just on the 550+space (in case anyone ever would come up with error 5501) as you suggested.

I also am stil considering whether we should include other numbers, or perhaps even do the reverse: there are only a few cases in which we actually want to do the delete.
Ideally we should check beforehand whether the file exists and overwriting is not allowed, but the FEAT string is identical whether overwriting is allowed or not.

{
delfile=0;
}

ec_print(ec);
ec_destroy(ec);

Expand Down
5 changes: 5 additions & 0 deletions errcode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ ec_destroy(errcode_t errcode)
return;
}

char
*ec_get_errmsg(errcode_t errcode)
{
return (errcode ? *(errcode->errmsg) : "");
}

void
ec_print(errcode_t errcode)
Expand Down
3 changes: 3 additions & 0 deletions errcode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@

typedef struct errcode_s * errcode_t;

char *
ec_get_errmsg(errcode_t errcode);

errcode_t
ec_create(OM_uint32, OM_uint32, char * fmt, ...);

Expand Down