On Sun, Jul 22, 2012 at 07:04:58PM +0200, Davidlohr Bueso wrote:
> fdisks/fdisk.c | 35 +++-------------
> fdisks/fdisk.h | 33 +++++++++-----
> fdisks/fdiskaixlabel.c | 4 +-
> fdisks/fdiskbsdlabel.c | 20 +++++-----
> fdisks/fdiskdoslabel.c | 4 +-
> fdisks/fdiskmaclabel.c | 4 +-
> fdisks/fdisksgilabel.c | 12 +++---
> fdisks/fdisksunlabel.c | 8 ++--
> fdisks/utils.c | 90 ++++++++++++++++++++++++++++++++++-------
> tests/expected/fdisk/oddinput | 4 +-
> 10 files changed, 131 insertions(+), 83 deletions(-)
Not applied. I think that we need to follow standard libc error
handling as much as possible rather than try to reimplement our own
universe.
> - cxt = fdisk_new_context_from_filename(argv[optind], 0);
> + cxt = fdisk_new_context_from_filename(argv[optind], 0, &errcode);
> if (!cxt)
> - err(EXIT_FAILURE, _("cannot open %s"), argv[optind]);
> + errx(EXIT_FAILURE, _("%s: %s"), fdisk_error_name(errcode),
> + argv[optind]);
This is exactly why I don't like it. There is errno and err() to
print details about the error, it's able to follow locales, etc.
Your FDISK_ERROR_OPEN is too generic and fdisk_error_name() have no
clue about locales.
The old err() and errno based implementation is able to provide
details like:
cannot open /dev/sda: Permission denied
or ENOMEM, etc.
I have fixed the new fdisk API to use errno as much as possible. For
now it seems that we does not need any extra error handling
infrastructure... so it will be better to postpone until we found any
real issue.
The basic rule is simple, just return -errno, for example:
if (syscal( .... ) == -1)
return -errno;
if no errno is set then use -E<something from man errno> for example:
if (!cxt)
return -EINVAL;
or (and) set the errno. If errno.h stuff is insufficient the we can
define our own codes (maximum system errno value should be 4095 on UNIXe)
so we can use values greater than 5000 or so).
I know that I have talked with David about fdisk_strerror() or so,
but it's seems like overkill for now. I'd like to avoid error
messages in the libfdisk library, it's better to use codes and
compose messages in programs.
> --- a/tests/expected/fdisk/oddinput
> +++ b/tests/expected/fdisk/oddinput
> @@ -9,6 +9,6 @@ Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
>
> Nonexistant file
> -lt-fdisk: unable to open _a_file_that_does_not_exist_: No such file or directory
> +lt-fdisk: error: cannot open: _a_file_that_does_not_exist_
> Too small file
> -lt-fdisk: unable to open oddinput.toosmall: Success
> +lt-fdisk: error: cannot read: oddinput.toosmall
Fixed by errno = EINVAL, the test updated ;-)
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
