[Gnu-arch-users] Suggestions for handling in pfs-sftp.c?

[Aaron Bentley]

Hi all,
I'm looking for suggestions about how to handle errors in pfs-sftp;
Recently, I was trying to create an archive, and got the mysterious 
error "Failure".

With the help of gdb, I determined that the messages was coming from 
sftp_decode_status, and the "failure" was "Failed to create 
already-existing directory".  Clearly, this isn't a very helpful 
message, but some of the other cases are better.  "Permission denied" is 
another possible output from sftp_decode_status().

At first, I thought the solution was to use the soft_errors flag, and 
handle the errors in pfs_mkdir.  But if you pass the soft_errors flag, 
you have no way of distinguishing between the several types of errors.  
(The list is documented in sftp.h) .

It's times like this that I miss C++ exceptions, and one approach would 
be a global error stack:

pfs_mkdir(...)
{
...
push_error("Error creating directory %s in directory %s", path,  pfs->cwd);
/**
 * any code that could fail here,
 */
pop_error(); // if we get here, no actual error
}

Then in sftp_decode_status(),
...
if (!soft_errors)
{
... 
 print_error_stack();
/* error handling as normal */
exit(2);
}

This would produce output like so:
Error creating archive baz:
Error creating directory foo in path bar:
Failure

What I like about messages like this is that they're factual, and they 
don't require functions to know anything beyond what they already know.

But it's a solution that would require changes at every level of the 
code, so I can understand why people would be reluctant to pursue it.  
Also it could lead to visible slowdowns, depending on implementation.

Other options include:
- A version of sftp_decode_status() that returns the status as a string
- A version of sftp_decode_status() that returns the status as a number, 
and an sftp-status-number-to-string function.
- Modifying pf_mkdir to call sftp_decode_status() once with soft_errors 
true, and once with it false, to print some context befor printing the 
error.
- Modifying pf_mkdir() or arch_pfs_pfs_make_archive() to test for the 
directory's existence before calling pf_mkdir()

Any comments on the best approach to take?

Aaron