No subject
Mon Jul 23 18:04:13 UTC 2007
before you create it!"
I undoubtedly can fix bbox cp to match GNU message, but code will be
bigger
and GNU message is still semi-stupid.
For the record: fix should be here:
static int ask_and_unlink(const char *dest, int flags)
{
...
if (unlink(dest) < 0) {
===> if errno == ENOENT or ENOTDIR, explain that *path* is invalid, not
filename <===
bb_perror_msg("cannot remove '%s'", dest);
return -1; // error
}
return 1; // ok (to try again)
}
----------------------------------------------------------------------
kiltedknight - 08-24-07 08:58
----------------------------------------------------------------------
I've read it again. You don't seem to understand...
If the path in which you want to create the file does not exist, you
should never see EEXIST... that implies the file already exists!
----------------------------------------------------------------------
vda - 08-24-07 09:55
----------------------------------------------------------------------
bbox cp says:
cp: cannot remove '/tmp/nonexistent/path/foo': No such file or directory
which sounds stupid
GNU coreutils cp says:
cp: cannot create regular file `/tmp/nonexistent/path/foo': No such file
or directory
which is also stupid.
Do you agree with me on the above?
If yes: then why we should change from one stupid message to another
stupid one?
If no: explain why do you think that coreutils message is better.
----------------------------------------------------------------------
kiltedknight - 08-24-07 10:05
----------------------------------------------------------------------
It's far less stupid to say, "No such file or directory" when part or all
of the path does not exist than to say, "cannot remove..." because the
user is trying to create a file, not remove one.
Also, try compiling with DO_POSIX_CP enabled as well... instead, you get
the message that the file exists when you try to copy to a path that
doesn't exist.
In all cases, the best choice of error messages is "No such file or
directory," which will clue the user in on what is actually wrong.
----------------------------------------------------------------------
vda - 08-24-07 10:08
----------------------------------------------------------------------
Re EEXIST:
} else if (S_ISBLK(source_stat.st_mode) ||
S_ISCHR(source_stat.st_mode)
|| S_ISSOCK(source_stat.st_mode) ||
S_ISFIFO(source_stat.st_mode)
|| S_ISLNK(source_stat.st_mode)
) {
// We are lazy here, a bit lax with races...
if (dest_exists) {
==> errno = EEXIST;
ovr = ask_and_unlink(dest, flags);
It is added here purely for the sake of bb_perror_msg inside
ask_and_unlink in "#if DO_POSIX_CP" block to print correct error code. It
will happen if you try to cp a device node, fifo, or symlink over existing
file without -i and -f.
static int ask_and_unlink(const char *dest, int flags)
{
#if DO_POSIX_CP
if (!(flags & (FILEUTILS_FORCE|FILEUTILS_INTERACTIVE))) {
// Either it exists, or the *path* doesnt exist
==> bb_perror_msg("cannot create '%s'", dest);
return -1;
}
#endif
It will say "cp: cannot create 'foo': File already exists".
----------------------------------------------------------------------
vda - 08-24-07 13:52
----------------------------------------------------------------------
kiltedknight, please test attached patch. It gives good error messages for
both ENOENT and ENOTDIR:
$ ./busybox cp busybox /qwe/asd
cp: cannot create '/qwe/asd': Path does not exist
$ ./busybox cp busybox /vmlinuz/asd
cp: cannot stat '/vmlinuz/asd': Path has non-directory component
svn busybox:
$ ./busybox cp busybox /qwe/asd
cp: cannot remove '/qwe/asd': No such file or directory
$ ./busybox cp busybox /vmlinuz/asd
cp: cannot stat '/vmlinuz/asd': Not a directory
Cost:
ask_and_unlink 81 119 +38
cp_mv_stat2 79 99 +20
----------------------------------------------------------------------
kiltedknight - 08-24-07 14:30
----------------------------------------------------------------------
Patch is working. Thank you. :)
Issue History
Date Modified Username Field Change
======================================================================
08-23-07 10:55 kiltedknight New Issue
08-23-07 10:55 kiltedknight Status new => assigned
08-23-07 10:55 kiltedknight Assigned To => BusyBox
08-23-07 10:55 kiltedknight File Added: bb_cp_errmsg_fix.diff
08-23-07 10:55 kiltedknight Issue Monitored: kiltedknight
08-24-07 07:22 vda Status assigned => closed
08-24-07 07:22 vda Note Added: 0002681
08-24-07 07:22 vda Resolution open => won't fix
08-24-07 08:58 kiltedknight Status closed => feedback
08-24-07 08:58 kiltedknight Resolution won't fix => reopened
08-24-07 08:58 kiltedknight Note Added: 0002682
08-24-07 09:55 vda Note Added: 0002683
08-24-07 10:05 kiltedknight Note Added: 0002684
08-24-07 10:08 vda Note Added: 0002685
08-24-07 13:48 vda File Added: 2.patch
08-24-07 13:52 vda Note Added: 0002686
08-24-07 14:30 kiltedknight Note Added: 0002687
======================================================================
More information about the busybox-cvs
mailing list