Bug-871: mv builtins cannot move on cross-mountpoint, cp obscure error #878
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
when -r is not given on src dir. - src/lib/libcmd/cp.c If src is a dir and -r was not given, warn with a message like -r not specified; omitting directory This fix bogus error message like cp: warning: t: directory -- copying as plain file cp: t: /tmp/t read error [Is a directory] This is not the exact purpose of the bug-871 submission, but it is in the same vein. - src/lib/libcmd/mv.c The builtin mv is unable to deal with cross-mounted file system. The code still has a -x option for 'may be' cross-mounted file system but it doesn't works. The effect of $ /opt/ast/bin/mv bin /dev/shm mv: bin: /dev/shm/bin read error [Is a directory] Is a failure on xdev mv, and a left over regular file (not dir) named. '/tmp/bin' Then $ /opt/ast/bin/cp -r bin /dev/shm cp: /dev/shm/bin: not a directory -- bin ignored Since /tmp/bin is a regular file we got the above error. The code is pretty obscure and wrong, to avoid upsetting this instable design, I propose a workaround instead of a fix. The idea is to detect xdev mounted target dst and do a cp -r src... dst If all goes well do rm -rf src... NOTE : The bug-871 report the problem with /tmp or /dev/shm but it is not related to those FS, using another disc (FS) fail the same way. The workaround. in b_mv() we try to figure out if some src... dev don't match the dts dev, if so we craft an argc argv and call b_cp() with argv[0]="XM", i.e a signature not used by b_cp() which can be called with the signature "cp", "ln", "mv". In b_cp() we detect the "XM" signature, and restore a valid "cp" signature, this signature is used by cmdinit() to set error_info.id="cp", i.e the basename of argv[0], as side effect, used for error reporting in "cp". After the cmdinit() we keep a backup of the cmd because the behavior of the code depend on switch (error_info.id[0]), and since our XM "cp" is actually a run on "mv" behalf, we set error_info.id to "mv" that is important for accurate error reporting, and now the switc become switch(cmd[0]) to still work as a cp but with an mv error report. Down the code, while looping on src... if the cmd[0]=='c' i.e cp mode, and src is a dir and -r was not given, we produce the more conventional error message "-r not specified; omitting directory %s",src
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
when -r is not given on src dir.
src/lib/libcmd/cp.c If src is a dir and -r was not given, warn with a message like -r not specified; omitting directory
This fix bogus error message like cp: warning: t: directory -- copying as plain file cp: t: /tmp/t read error [Is a directory]
This is not the exact purpose of the bug-871 submission, but it is in the same vein.
src/lib/libcmd/mv.c The builtin mv is unable to deal with cross-mounted file system. The code still has a -x option for 'may be' cross-mounted file system but it doesn't works.
The effect of $ /opt/ast/bin/mv bin /dev/shm mv: bin: /dev/shm/bin read error [Is a directory]
Is a failure on xdev mv, and a left over regular file (not dir) named. '/tmp/bin'
Then $ /opt/ast/bin/cp -r bin /dev/shm cp: /dev/shm/bin: not a directory -- bin ignored Since /tmp/bin is a regular file we got the above error.
The code is pretty obscure and wrong, to avoid upsetting this instable design, I propose a workaround instead of a fix.
The idea is to detect xdev mounted target dst and do a cp -r src... dst If all goes well do rm -rf src...
NOTE : The bug-871 report the problem with /tmp or /dev/shm but it is not related to those FS, using another disc (FS) fail the same way.
The workaround.
in b_mv() we try to figure out if some src... dev don't match the dts dev, if so we craft an argc argv and call b_cp() with argv[0]="XM", i.e a signature not used by b_cp() which can be called with the signature "cp", "ln", "mv".
In b_cp() we detect the "XM" signature, and restore a valid "cp" signature, this signature is used by cmdinit() to set error_info.id="cp", i.e the basename of argv[0], as side effect, used for error reporting in "cp".
After the cmdinit() we keep a backup of the cmd because the behavior of the code depend on switch (error_info.id[0]), and since our XM "cp" is actually a run on "mv" behalf, we set error_info.id to "mv" that is important for accurate error reporting, and now the switc become switch(cmd[0]) to still work as a cp but with an mv error report.
Down the code, while looping on src... if the cmd[0]=='c' i.e cp mode, and src is a dir and -r was not given, we produce the more conventional error message
"-r not specified; omitting directory %s",src