Bug 202424

Summary:

[regression] fetch(1) reports "0 B 0 Bps" in the logs

Product:

Base System

Reporter:

Alexey Dokuchaev <danfe>

Component:

bin

Assignee:

Dag-Erling Smørgrav <des>

Status:

Closed FIXED

Severity:

Affects Many People

CC:

des

Priority:

---

Keywords:

regression

Version:

CURRENT

Flags:

des: mfc-stable11+
des: mfc-stable10-

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
Fix end-of-transfer statistics when not on a tty	none
Fix end-of-transfer statistics when not on a tty	none
Fix end-of-transfer statistics when not on a tty	none

Description Alexey Dokuchaev freebsd_committer

2015-08-18 19:50:03 UTC

I've observed that with newer FreeBSD releases, fetch(1) would leave bogus transfer statistics in the logs (that is, when it's being redirected):

> $ uname -rms
> FreeBSD 11.0-CURRENT i386
> $ fetch http://homehost/somefile >& log
> $ cat log 
> somefile                                                 0  B    0  Bps

Same happens on 10.X releases.  While on 8.4, the output correctly lists file length and transfer speed:

> $ uname -rms
> FreeBSD 8.4-STABLE i386
> $ fetch http://homehost/somefile >& log
> $ cat log
> somefile                                               178 kB   11 MBps

This bugs occurs independently of the shell being used (sh/tcsh/bash).

Comment 1 Alexey Dokuchaev freebsd_committer

2015-08-18 20:04:27 UTC

Interestingly, on FreeBSD 9.2-RELEASE, the file length is reported correctly, but transfer speed is always 0 Bps.  But on FreeBSD 9.3-RELEASE, both numbers are zeros.

Comment 2 Alexey Dokuchaev freebsd_committer

2018-08-30 01:19:01 UTC

It was reported back in whew 11.x was -CURRENT, but it affects recent FreeBSD versions just as well.  Please, next time, do the proper checks before modifying affected version of the PR, thank you.

Comment 3 Dag-Erling Smørgrav freebsd_committer

2018-09-06 12:52:27 UTC

Created attachment 196911 [details]
Fix end-of-transfer statistics when not on a tty

Please try the attached patch ASAP so I can get it in before 12.  In addition to fixing the bug (caused by insufficient initialization of struxt xferstats and reuse of a static buffer), it also cleans up the code and improves the output for both tty and non-tty cases.

Comment 4 Dag-Erling Smørgrav freebsd_committer

2018-09-06 12:57:21 UTC

Created attachment 196912 [details]
Fix end-of-transfer statistics when not on a tty

Remove unrelated hunk from the patch.

Comment 5 Alexey Dokuchaev freebsd_committer

2018-09-10 16:04:53 UTC

Yes, it mostly fixes the problem, but not for 100%.  I had to build with -DNO_WERROR on i386 because of the following warnings (several of them):

> /usr/src/usr.bin/fetch/fetch.c:212:7: warning: format specifies type 'long' but
>       the argument has type 'int' [-Wformat]
>                     seconds / 86400, (seconds % 86400) / 3600);
On amd64, it builds cleanly, but statistics is strange when redirecting to a file while being fine on tty:

> /tmp/usr/src/amd64.amd64/usr.bin/fetch/fetch http://www.ru
> fetch: http://www.ru: size of remote file is not known
> www.ru                                                2854  B   21 MBps    00s
> 
> /tmp/usr/src/amd64.amd64/usr.bin/fetch/fetch http://www.ru >& /tmp/foo
> cat /tmp/foo
> fetch: http://www.ru: size of remote file is not known
> www.ru                                                184467440737095   28 MBps    00s

Comment 6 Dag-Erling Smørgrav freebsd_committer

2018-09-10 17:30:06 UTC

The build issue on i386 comes from using "%ld" for time_t, even though they are the same size.  The other issue comes from xs->size being -1 when the remote size is unknown; I didn't test that scenario.  I will post a new patch later tonight.

Comment 7 Alexey Dokuchaev freebsd_committer

2018-09-10 17:42:47 UTC

> The build issue on i386 comes from using "%ld" for time_t, even though they
> are the same size.
Sure, it's pretty obvious.  Just had to let you know.

> The other issue comes from xs->size being -1 when the remote size is unknown
Indeed, when size is reported by the server, statistics are identical and correct (file vs. tty).

Comment 8 Dag-Erling Smørgrav freebsd_committer

2018-09-10 17:58:02 UTC

Created attachment 197011 [details]
Fix end-of-transfer statistics when not on a tty

Fixed the i386 issue and the end-of-transfer underflow when not on a tty, please test again.

Comment 9 Alexey Dokuchaev freebsd_committer

2018-09-10 18:30:26 UTC

Thanks, looks like both issues are fixed now.  Tested on i386, amd64, and powerpc.

Comment 10 commit-hook freebsd_committer

2018-09-10 19:39:48 UTC

A commit references this bug:

Author: des
Date: Mon Sep 10 19:39:21 UTC 2018
New revision: 338572
URL: https://svnweb.freebsd.org/changeset/base/338572

Log:
  Through a combination of insufficient variable initialization and
  imprudent reuse of static buffers, the end-of-transfer statistics
  displayed when stdout is not a tty always ended up as 0 B / 0 Bps.
  Reorganize the code to use caller-provided buffers, tweak the ETA
  display a bit, and reduce the visual differences between the tty and
  non-tty end-of-transfer displays.

  PR:		202424
  Approved by:	re (gjb@)

Changes:
  head/usr.bin/fetch/fetch.c

Comment 11 commit-hook freebsd_committer

2018-10-09 10:49:35 UTC

A commit references this bug:

Author: des
Date: Tue Oct  9 10:49:21 UTC 2018
New revision: 339250
URL: https://svnweb.freebsd.org/changeset/base/339250

Log:
  MFH (r314778): use reallocarray(3) for extra bounds checks
  MFH (r333306): fix typo in man page
  MFH (r333571, r333572): preserve if-modified-since across redirects
  MFH (r334317): simplify the DEBUG macro
  MFH (r334319): style bug roundup
  MFH (r334326): fix netrc file location logic, improve netrcfd handling
  MFH (r338572): fix end-of-transfer statistics, improve no-tty display

  PR:		202424, 224426, 228017

Changes:
_U  stable/11/
  stable/11/lib/libfetch/common.c
  stable/11/lib/libfetch/common.h
  stable/11/lib/libfetch/fetch.c
  stable/11/lib/libfetch/ftp.c
  stable/11/lib/libfetch/http.c
  stable/11/usr.bin/fetch/fetch.1
  stable/11/usr.bin/fetch/fetch.c