On Fri, 2014-02-28 at 11:23 -0500, Phillip Susi wrote:
> On 2/28/2014 6:00 AM, Maciej Małecki wrote:
> >>> libfdisk/src/context.c | 17 +++++++++++++---- 1 file changed,
> >>> 13 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/libfdisk/src/context.c b/libfdisk/src/context.c
> >>> index c405403..40f9080 100644 --- a/libfdisk/src/context.c +++
> >>> b/libfdisk/src/context.c @@ -248,17 +248,26 @@ static int
> >>> warn_wipe(struct fdisk_context *cxt) int
> >>> fdisk_context_assign_device(struct fdisk_context *cxt, const
> >>> char *fname, int readonly) { - int fd; + int fd, mode;
> >>>
> >>> DBG(CONTEXT, dbgprint("assigning device %s", fname));
> >>> assert(cxt);
> >>>
> >>> reset_context(cxt);
> >>>
> >>> - if (readonly == 1 || (fd = open(fname, O_RDWR|O_CLOEXEC))
> >>> < 0) { - if ((fd = open(fname, O_RDONLY|O_CLOEXEC))
> >>> < 0) +retry: + mode = (readonly ? O_RDONLY : O_RDWR) |
> >>> O_CLOEXEC; + fd = open(fname, mode); + if (fd < 0) { +
> >>> if (readonly) return -errno; - readonly = 1; +
> >>> else { + fdisk_warn(cxt, +
> >>> _("%s: opening device in read write mode failed"), +
> >>> fname); + readonly = 1; +
> >>> goto retry; + } }
> >>
> >> This is 1) obscenely ugly and 2) not the place for such message.
> >> There's no reason to pollute output like this. Why do I care how
> >> the device is opened when I'm not going to write anything to it?
> >>
> >> The correct way is to do what you do in patch 2, inform the user
> >> that the device is only for opened for reading and just skip the
> >> write operation.
>
> You really want to tell the user that they aren't going to be able to
> write up front.
Sure why not, just not in a library! You can do this in the fdisk
program. And if Karel doesn't agree and wants to apply this patch
anyway, then it should *at least* be in DBG context.
Thanks,
Davidlohr
--
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
