| Summary: | [regression, patch] 1.18.x wget breaks with script-generated output due to not considering errno=EAGAIN | ||
|---|---|---|---|
| Product: | Busybox | Reporter: | William Pitcock <nenolod> |
| Component: | Networking | Assignee: | unassigned |
| Status: | RESOLVED FIXED | ||
| Severity: | minor | CC: | busybox-cvs |
| Priority: | P5 | ||
| Version: | 1.18.x | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Host: | Target: | ||
| Build: | |||
| Attachments: | patch to ignore EAGAIN as well as EINTR. | ||
|
Description
William Pitcock
2011-02-11 19:05:43 UTC
Created attachment 2959 [details]
patch to ignore EAGAIN as well as EINTR.
EINTR is no longer possible since progress meter does not use signals anymore. Therefore fgets/fread wrappers will be deleted in the next release. I don't understand under what conditions EAGAIN is a problem. Here is a test script I use: file=debian-31r0a-i386-binary-1.iso >$file dd bs=1M count=639 </dev/zero >$file strace -tt -oLOG ./busybox wget -c ftp://ftp.uar.net/pub/Linuxes/debian3.1/$file And in the LOG, I see: 21:10:17.232823 poll([{fd=6, events=POLLIN|POLLPRI, revents=POLLIN}], 1, 1000) = 1 21:10:17.234052 read(6, "\216\177\231\241\6H>tp\3713\177D.\37\237\313csy|.O\315"..., 65536) = 1452 21:10:17.234123 read(6, 0x8110e1c, 61440) = -1 EAGAIN (Resource temporarily unavailable) 21:10:17.234172 write(3, "\216\177\231\241\6H>tp\3713\177D.\37\237\313csy|.O\315"..., 1452) = 1452 21:10:17.234224 clock_gettime(0x1 /* CLOCK_??? */, {2322, 135332047}) = 0 21:10:17.234265 poll([{fd=6, events=POLLIN|POLLPRI, revents=POLLIN}], 1, 1000) = 1 21:10:17.235794 read(6, "\344\2677\303\344\30\20\317u\256\344%\344\331\222\333\310"..., 65536) = 1452 21:10:17.235865 read(6, 0x8110e1c, 61440) = -1 EAGAIN (Resource temporarily unavailable) 21:10:17.235907 write(3, "\344\2677\303\344\30\20\317u\256\344%\344\331\222\333\310"..., 1452) = 1452 21:10:17.235960 clock_gettime(0x1 /* CLOCK_??? */, {2322, 137068378}) = 0 21:10:17.236002 poll([{fd=6, events=POLLIN|POLLPRI, revents=POLLIN}], 1, 1000) = 1 21:10:17.237424 read(6, "\272\237\237\323uw\345\375\232*_\357\361/\27z\354\257\374"..., 65536) = 1452 21:10:17.237495 read(6, 0x8110e1c, 61440) = -1 EAGAIN (Resource temporarily unavailable) 21:10:17.237537 write(3, "\272\237\237\323uw\345\375\232*_\357\361/\27z\354\257\374"..., 1452) = 1452 21:10:17.237615 clock_gettime(0x1 /* CLOCK_??? */, {2322, 138722873}) = 0 As you see, EAGAIN does happen, but it doesn't break download. It merely makes fread(buf, 1, size, fp) return with partial read (instead of blocking). I tested glibc and uclibc and both work fine. Can you show your strace log of the bug in action? hello, this patch does not work. consider the following URL: http://git.alpinelinux.org/cgit/aports.git/snapshot/aports-2.1.4.tar.bz2 the patch i provide makes this URL work, the other patch does not. The patch you provide modifies safe_fgets function, which is deleted altogether in the development version. I want to have the same fix in 1.18.x and in current git. You are right, wgetting http://git.alpinelinux.org/cgit/aports.git/snapshot/aports-2.1.4.tar.bz2 doesn't work. I figured out what's wrong and updated the patch: http://busybox.net/downloads/fixes-1.18.3/busybox-1.18.3-wget.patch Please try it again. In my testing, now it works. hi, this patch is a slight improvement over the last one, but it's broken, as it only downloads the first few bytes of the tarball. petrie:~/aports/main/busybox$ sudo apk add ./busybox-1.18.2-r3.apk (1/1) Upgrading busybox (1.18.2-r2 -> 1.18.2-r3) Executing busybox-1.18.2-r3.post-upgrade Executing busybox-1.18.2-r3.trigger OK: 611 packages, 4538 dirs, 84944 files petrie:~/aports/main/busybox$ wget http://git.alpinelinux.org/cgit/aports.git/snapshot/aports-2.1.4.tar.bz2 Connecting to git.alpinelinux.org (91.220.88.36:80) aports-2.1.4.tar.bz2 100% |********************************************************************************************************************************************************************************************| 5 --:--:-- ETA petrie:~/aports/main/busybox$ ls -al aports-2.1.4.tar.bz2 -rw-r--r-- 1 nenolod nenolod 5 Feb 11 15:20 aports-2.1.4.tar.bz2 petrie:~/aports/main/busybox$ file aports-2.1.4.tar.bz2 aports-2.1.4.tar.bz2: ASCII text, with CRLF line terminators petrie:~/aports/main/busybox$ (In reply to comment #6) > hi, > > this patch is a slight improvement over the last one, but it's broken Actually, it is not an improvement at all, because I forgot to *upload* the improved patch to the server :( Sorry. Uploaded now. Here's how I tested it: set -e wget http://busybox.net/downloads/busybox-1.18.3.tar.bz2 wget http://busybox.net/downloads/fixes-1.18.3/busybox-1.18.3-wget.patch tar xf busybox-1.18.3.tar.bz2 cd busybox-1.18.3 patch -p1 <../busybox-1.18.3-wget.patch make defconfig make ./busybox wget http://git.alpinelinux.org/cgit/aports.git/snapshot/aports-2.1.4.tar.bz2 ls -l aports-2.1.4.tar.bz2 bunzip2 -t aports-2.1.4.tar.bz2 echo Test result: $? Result: ... ... CC util-linux/volume_id/util.o CC util-linux/volume_id/volume_id.o CC util-linux/volume_id/xfs.o AR util-linux/volume_id/lib.a LINK busybox_unstripped Trying libraries: crypt m Library crypt is not needed, excluding it Library m is needed, can't exclude it (yet) Final link with: m DOC busybox.pod DOC BusyBox.txt DOC busybox.1 DOC BusyBox.html Connecting to git.alpinelinux.org (91.220.88.36:80) aports-2.1.4.tar.bz2 100% |************************| 1092k --:--:-- ETA -rw-r--r-- 1 root root 1566380 Feb 11 22:52 aports-2.1.4.tar.bz2 Test result: 0 hi, indeed this patch works as expected. i've gone ahead and replaced my patch with yours in the busybox-1.18.3-r0 update for Alpine i am about to push. as far as i am concerned, this bug can be closed out :) Fixed, fix will be in 1.18.4 and 1.19.x |