Simple fix for github issue #3

[msalle]

This is a simple and not very clean fix for
/issues/3
Either (one of) the l_write() or the l_close() can fail if the remote file
already exists, since the data goes over a different channel than the control
messages. For large files, it will typically be one of the later l_write()
statements, for small files probably the l_close(). Hence we effectively only
have the errmsg to find out what happened, and therefore do a simple str(n)cmp
with this errmsg. For this we also need to add a function to get it from the
opaque errcode_t.
Note that there are other FTP error codes that probably should prevent the delete.
Feedback on this would be highly appreciated (-;

[matyasselmeci]

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)

[msalle]

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.

[fscheiner]

@msalle:
Is the current state of the patches up to date with what was discussed/decided in #3?

And do the changes work as expected during a test replicating the situation described in the original GGUS ticket?

[msalle]

@msalle:
Is the current state of the patches up to date with what was discussed/decided in #3?

No, this PR is still in the state where it would only NOT delete when the failure is a 550. For other failure modes it would still follow the code leading to the original deletion of the remote file.
I have an additional branch https://github.com/msalle/UberFTP/tree/no_delete_file / msalle@5205407 with almost trivial change just disabling (with an #if 0 to keep the original code around) the deletion. No pull request from that one (yet).
Paul's intermediate solution in #3 (comment) would be nicer, but difficult to implement.

And do the changes work as expected during a test replicating the situation described in the original GGUS ticket?

Yes, I've extensively tested it with a locally built Uberftp from this branch and both prometheus.desy.de and gridftp.grid.sara.nl.

[fscheiner]

@msalle:
Well, I'd go for the simple change then, though I'd just remove the relevant code, instead of masking it. I mean, it's still kept in the history, so no need to keep it around.

@matyasselmeci @ellert
What would you suggest?

[msalle]

I'm closing this PR in favour of #5