Bug 186456 - Copy text file to a ssh vfs add garbage at the top of the file
Summary: Copy text file to a ssh vfs add garbage at the top of the file
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: mc
Version: 5
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jindrich Novy
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-03-23 18:03 UTC by Otto Rey
Modified: 2013-07-02 23:15 UTC (History)
4 users (show)

Fixed In Version: mc-4.6.1a-13.FC5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-06-14 09:15:59 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Pipe error output from head to /dev/null (735 bytes, patch)
2006-03-24 16:40 UTC, Leonard den Ottolander
no flags Details | Diff
Full fish-upload patch v2: handle Solaris (2.81 KB, patch)
2006-03-27 12:27 UTC, Dmitry Butskoy
no flags Details | Diff
Fish upload patch: final variant (3.04 KB, patch)
2006-03-29 11:27 UTC, Dmitry Butskoy
no flags Details | Diff

Description Otto Rey 2006-03-23 18:03:26 UTC
Description of problem:
When copy text file to a ssh vfs, some garbage (like a help of a command) is
appended at the top of the destination file. The garbage is: "usage: head [-n #]
[-#] [filename...]"

Version-Release number of selected component (if applicable): FC5 - MC


How reproducible:


Steps to Reproduce:
1. Connect to another machine with ssh
2. Copy a local filesystem text-file to the ssh VFS
3. Open remote text file and see the garbage a the top of the file.
  
Actual results:
The copied text file have garbage (usage: head [-n #] [-#] [filename...])

Expected results:
The file copied correctly

Additional info:
Connecting to Solaris. Only in text files. The garbage sounds like a help for a
command:

"usage: head [-n #] [-#] [filename...]"

Comment 1 Jindrich Novy 2006-03-24 13:46:47 UTC
I suspect that Solaris uses a non-GNU implementation of head so it lacks some
options that FISH vfs in mc requires for communication. I'm quite sure about
this because the help of GNU head looks like this:

Usage: head [OPTION]... [FILE]...
Print the first 10 lines of each FILE to standard output.
With more than one FILE, precede each with a header giving the file name.
With no FILE, or when FILE is -, read standard input.

In the code I found:

    /* First, try this as stor:
     *
     *     ( head -c number ) | ( cat > file; cat >/dev/null )
     *
     *  If `head' is not present on the remote system, `dd' will be used.
     * Unfortunately, we cannot trust most non-GNU `head' implementations
     * even if `-c' options is supported. Therefore, we separate GNU head
     * (and other modern heads?) using `-q' and `-' . This causes another
     * implementations to fail (because of "incorrect options").
     *
     *  Fallback is:
     *
     *     rest=<number>
     *     while [ $rest -gt 0 ]
     *     do
     *        cnt=`expr \( $rest + 255 \) / 256`
     *        n=`dd bs=256 count=$cnt | tee -a <target_file> | wc -c`
     *        rest=`expr $rest - $n`
     *     done
     *
     *  `dd' was not designed for full filling of input buffers,
     *  and does not report exact number of bytes (not blocks).
     *  Therefore a more complex shell script is needed.
     */

So you can eventually use this fallback. Hope that helps.

Since this is not a bug in Fedora, closing NOTABUG.

Comment 2 Leonard den Ottolander 2006-03-24 15:32:44 UTC
Jindrich, which code are you referring to when stating "In the code I found:"?

Comment 3 Leonard den Ottolander 2006-03-24 16:32:39 UTC
That usage of head is in FC's fish upload patch. Original code only uses dd
afaict. Reopening.

You might use "2> /dev/null" to get rid of the error generated by head on Solaris.

Comment 4 Leonard den Ottolander 2006-03-24 16:40:28 UTC
Created attachment 126654 [details]
Pipe error output from head to /dev/null

Pipe error output from head to /dev/null. This patch should be applied against
the fish upload patch.

Otto, does this fix your issue?

Comment 5 Leonard den Ottolander 2006-03-24 16:48:34 UTC
Hm. I now see the error output of head already is piped to /dev/null (duh).
Maybe Solaris' head behaves oddly with respect to this?

I have no access to Solaris. Otto, maybe you could figure out how to pipe the
head error output to /dev/null and let us know?


Comment 6 Otto Rey 2006-03-25 03:36:36 UTC
Ummm... i think that >/dev/null is the correct way to pipe the head error output
to null in Solaris too. I dont have Solaris rigth now to try this. I try it later.

What about this line:
head -c %lu -q - || echo DD >&3\n 


This one may cause the problem?

Comment 7 Jindrich Novy 2006-03-26 17:54:48 UTC
Hi Leonard, the note I presented here is in vfs/fish.c in fish_file_store().
Confirmed it comes from the Dmitry's patch (bug 140750) so adding him here to
join the discussion.

Comment 8 Jindrich Novy 2006-03-26 18:03:45 UTC
Hi Otto,

comment #6: yes, this is likely the source of problem seen on solaris since the
non-GNU implementations lacks the -c option. In the current situation I'd better
not apply the speedup patch by default but make it optional by a macro in spec
file (disabled by default) so that mc user can recompile mc when he's sure that
he accesses only boxes with GNU head installed and want to use the FISH transfer
speedup.

Comment 9 Otto Rey 2006-03-26 20:21:23 UTC
Ok. I think another solution: when connect to ssh, "test" if -c option is
available and set some "global" flag. When starting copy choose (checking the
flag) if apply the fish transfer speedup or not. It is posible? This is to avoid
recompile, specialy for end users.

Comment 10 Leonard den Ottolander 2006-03-26 20:37:50 UTC
From what I can see the problem is that Solaris' head does not write its error
output to stderr but to stdout instead (this needs to be confirmed though). I
believe this is why the 2> /dev/null fails and the error output ends up in the
file. If my guess is right the problem is Solaris' head, not the discussed patch.


Comment 11 Dmitry Butskoy 2006-03-27 10:27:05 UTC
Well, guys, I am here.

> In the current situation I'd better not apply the speedup patch by default

Jindrich, don't hurry up to make a decision - we shall think up something. :)

Anyway, this patch is not "some speedup" patch, it is "speedup in several times"
patch. On the other side, the Solaris issue IMHO is a corner case. But I hope we
can find a solution here...


Otto,
When you get access to some Solaris system, could you first run some tests:

1) 
  echo foobar | head -c 1 2>/dev/null
to make sure "head" reports to stdout instead of stderr (comment #10),

2)
  echo foobar | head -c 1 >/dev/null 2>&1 ; echo $?
to check what is an exit status in this case.

3) What a vesrion of Solaris causes this issue? Is it OpenSolaris? Another
words, can I obtain someway a source code of its "head" implementation?


Note, that the patch discussed was checked on some non-GNU-head systems (IBM
AIX, some old SYSV etc), i.e. it is not a "all non-GNU-head problem", it is just
"Solaris head" problem.


Comment 12 Dmitry Butskoy 2006-03-27 11:48:01 UTC
I've obtained OpenSolaris source, and now I see the problem. (I hope the
"Closed" Solaris has the same or similar source too :)).

On any cmdline error the Solaris "head" call a "Usage()" function, which is:

static void
Usage()
{
        (void) printf(gettext("usage: head [-n #] [-#] [filename...]\n"));
        exit(1);
}

i.e., it writes to stdout instead of stderr. Urhhh...
Fortunately, they return good exit status.

Unfortunately, the source I had obtained is some recent version (20 march) -- it
can mean that the issue exists in all prior versions too, and even in the ALL
Solaris variants...


Otto,
You can skip the tests I wrote -- IMHO all is clean now.

Comment 13 Dmitry Butskoy 2006-03-27 12:27:34 UTC
Created attachment 126813 [details]
Full fish-upload patch v2: handle Solaris

OK, solution is found.

When upload, use ">/%s" in the DD fallback code to zero the target file anyway.

Previously ">/%s" was used before the whole code, which was extra for the
"head" way (as it always do "cat ... >/%s") and was not robust for DD fallback
(which was showed in Solaris). Now all should be robust.

For "file appending" variant (the second half of the patch) we cannot do
">/%s", as it zeroes the target file. It seems we cannot use head here at
all...
But "appending to target file" is obviously the corner case, therefore I assume
we can drop "head" usage for "appending" variant and just use DD-fallback for
it.


Otto,
Could you check this patch against Solaris?
(Just replace the original one in the MC source rpm).

Comment 14 Jindrich Novy 2006-03-27 12:53:43 UTC
Hi Dmitry,

I'll wait for Otto's response and then apply your newer version of the patch.

Thanks for the quick reply ;)

Comment 15 Otto Rey 2006-03-27 14:27:17 UTC
Confirmed: the patch is cool! :) this work very well!!

Tested with:
SunOS iwaylab 5.9 Generic_117171-15 sun4u sparc SUNW,Sun-Fire-V240

Congratulations!!

Thanks to all!

Comment 16 Dmitry Butskoy 2006-03-27 14:48:23 UTC
Otto,

Just to be sure: Do you have checked both upload-"creation/overwrite" and
upload-"append" ? (I'm paranoid a little :))

As you did some works anyway, could you test this patch against some more
non-GNU-head systems? (i.e., not Solaris, or some another version of Solaris
etc.)? While all the things are hot, it is a good moment for this... :)




Comment 17 Otto Rey 2006-03-27 15:57:21 UTC
Yes, i have checked create, overwrite and append and works ok.

I can't test against others non-GNU-head systems because i don't have more ;)

Comment 18 Leonard den Ottolander 2006-03-28 22:21:45 UTC
Shouldn't this behaviour of Solaris' head be reported as a bug with Sun?

Maybe the patch should be extended with a comment explaining the "> /%s\n" is
needed because of a faulty head implementation on Solaris?


Comment 19 Dmitry Butskoy 2006-03-29 11:24:09 UTC
> Shouldn't this behaviour of Solaris' head be reported as a bug with Sun?

Perhaps nope...
Look into the head.c file (you can obtain it from the on-src-YYYYMMDD.tar.bz2
tarball from the OpenSolaris site). It seems to me that it is not just a code of
Sun, IMHO it began since "1984 AT&T" time...
In other words, it seems that this bug can exist in another UN*Xes too, and
already exists there long time.

In such a situation the fixing of this bug by Sun for OpenSolaris will not take
much sense, as all the previous Sun systems, as well as some another UNIX
systems, are affected too. Anyway we should reckon with such systems in this patch.

> Maybe the patch should be extended with a comment explaining the "> /%s\n" is
> needed because of a faulty head implementation on Solaris?
Well, You're right.
"> /%s\n" is needed for DD-fallback in any case, but not using of head for the
file append case should be commented.



Comment 20 Dmitry Butskoy 2006-03-29 11:27:12 UTC
Created attachment 126971 [details]
Fish upload patch: final variant

with an additional comment added

Comment 21 Pavel Tsekov 2006-03-31 06:40:07 UTC
Dmitry pointed me to this discussion since I was not aware that a newer version
of the fish upload patch exists. Since I have applied the older patch upstream
yesterday I'll apply this one too.

Btw. if someone needs access to Solaris system for developing / testing MC under
Solaris just drop me a letter. I own a Solaris box and I'll add acount for you.


Comment 22 Leonard den Ottolander 2006-06-14 09:15:59 UTC
Issue is fixed in FC5.

Closing again. Changes since June 8th got lost due to a bugzilla crash.



Note You need to log in before you can comment on or make changes to this bug.