Reuben Thomas wrote:
> Ping! I've not heard anything about this; my current patch (as used in
> GNU Zile) still seems to work.
Hi Reuben,
Thanks for the reminder.
A couple of suggestions:
ERR_READ may be too generic, risking collision with symbols defined by
other applications. Perhaps names like GL_COPY_READ_FAILURE,
GL_ACL_PRESERVE_FAILURE, etc.
Regarding the tests, it'd be nice if your added test code
could exercise a few of the newly-exposed error return values.
I.e., try to copy an unreadable file and assert that you get
the right error code. Try to copy to a read-only file and
ensure you get a different one. Try to copy-with-backup in such
a way that creating or writing the backup file fails.
One more thing, I noticed that your ERR_ACL case does not
print anything. I suppose that is just to remain completely
compatible with existing code? It seems like it'd be better
to diagnose that failure, too.
Once you have a test module, please repost.
Thanks for doing this.
> On 2 August 2011 18:31, Reuben Thomas <r...@sc3d.org> wrote:
>> Here's a revised patch to provide error returning copy-file as well as
>> error-raising copy-file.
>>
>> Of particular interest:
>>
>> 1. What would you like to call the new function?
>>
>> 2. I haven't yet added a test; from looking at the tests for copy-file
>> it seems it should suffice to add a call to remove followed by an
>> asserted call to copy_file_preserving_error (or whatever it's called)
>> after the call to copy_file_preserving, i.e. just repeat the copy with
>> the other version of the function.
>>
>> diff --git a/lib/copy-file.c b/lib/copy-file.c
>> index f9cd9c0..4ae8427 100644
>> --- a/lib/copy-file.c
>> +++ b/lib/copy-file.c
>> @@ -53,47 +53,79 @@
>>
>> enum { IO_SIZE = 32 * 1024 };
>>
>> -void
>> -copy_file_preserving (const char *src_filename, const char *dest_filename)
>> +enum {
>> + ERR_OPEN_READ = 1,
>> + ERR_OPEN_BACKUP_WRITE,
>> + ERR_READ,
>> + ERR_WRITE,
>> + ERR_AFTER_READ,
>> + ERR_ACL
>> +};
>> +
...
>> +void
>> +copy_file_preserving (const char *src_filename, const char *dest_filename)
>> +{
>> + switch (_copy_file_preserving (src_filename, dest_filename))
>> + {
>> + case 0:
>> + return;
>> +
>> + case ERR_OPEN_READ:
>> + error (EXIT_FAILURE, errno, _("error while opening \"%s\" for
>> reading"),
>> + src_filename);
>> +
>> + case ERR_OPEN_BACKUP_WRITE:
>> + error (EXIT_FAILURE, errno, _("cannot open backup file \"%s\"
>> for writing"),
>> + dest_filename);
>> +
>> + case ERR_READ:
>> + error (EXIT_FAILURE, errno, _("error reading \"%s\""), src_filename);
>> +
>> + case ERR_WRITE:
>> + error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
>> +
>> + case ERR_AFTER_READ:
>> + error (EXIT_FAILURE, errno, _("error after reading \"%s\""),
>> src_filename);
>> +
>> + case ERR_ACL:
>> + exit (EXIT_FAILURE);
>> +
>> + default:
>> + exit (EXIT_FAILURE);
>> + }
