Re: tar-1.26 not unpacking archives with symlinks?
On Tuesday 13 March 2012 17:40:38 Natanael Copa wrote: > I have seen this too and it only happens when the symlink is extracted > before the file it points to. > > To reproduce: > touch myfile > ln -s myfile mylink > tar -cf testcase1.tar mylink myfile i vaguely recall seeing this before and the issue came down to tar not detecting the right features. might want to compare the config.log output between glibc and uclibc to see how they differ. -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 11:15:32PM +0100, Laurent Bercot wrote: > >>> alarm(1); > >>> fgets(buf, sizeof buf, f); > > > > I was assuming the old standard idiom of installing a do-nothing > > signal handler (without SA_RESTART) for SIGALRM so that fgets would > > return with an error and the program could proceed. > > Ugh. I get it. In this case, then, *of course* you want to get EINTR > reported to the user. And I already agreed that the libc should not > safe-wrap system calls, so yes, naturally fgets should be interrupted. Yes, this is the sort of usage case I was talking about all along, and as far as I can tell, really the only reason anybody would ever want EINTR in the first place. Otherwise it's just a nuisance. > But honestly, this seems to me like the lazy (in a bad sense) way to > write such a program; the right way would be to write a little > asynchronous select/poll loop with a timeout of 1 second and a reading > event on fd 0. The main reason why the latter paradigm isn't used as > much is that stdio doesn't handle partial reads and so is quite unusable > in asynchronous loops - in other words, stdio sucks. There are at least 3 ways to do it. You, and many people these days, favor event-driven programming with select/poll/libevent/whatever. This is certainly one reasonable design, but it tends to make things that would otherwise be simple rather complicated, and precludes using stdio. I wouldn't say this means stdio sucks; rather, it's just meant for other types of programming. The other 2 ways are the one I described (using EINTR) and threads with thread cancellation as the method for getting out of blocked situations. Both of these work quite well with stdio. > Whenever I need to listen to signals as events (or, in the case of > alarm(), as timeouts), I use a self-pipe and a select loop so I can > listen to any number of events at the same time. It's a bit harder to > program, but much more robust; the "blocking system calls + EINTR" > paradigm is ad-hoc and error-prone. It can break so easily, for instance > when your libc is misguided and safewraps around EINTR. ;) Well I would call this not just misguided but wrong since POSIX specifies that fgetc/fputc (and all other stdio functions defined in terms of them) return failure with errno==EINTR for interrupting signals... Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 05:07:32PM -0400, Mike Frysinger wrote: > On Tuesday 13 March 2012 16:56:13 u-uclibc-q...@aetey.se wrote: > > On Tue, Mar 13, 2012 at 04:24:33PM -0400, Mike Frysinger wrote: > > > > It should definitely not be added near the > > > > beginning of the structure, only past elements that could be part of > > > > the macro ABI. > > > > > > that would help only if the uClibc code itself had versioned functions > > > which handled the growing struct. > > > > Curious, why wouldn't this work without versioned functions? > > if end user code encodes sizeof(FILE) anywhere, they're creating a memory > region of a fixed number of bytes, so increasing the structure means that the > app will give us a pointer to memory of X bytes, but the library requires it > to be Y (where Y is greater than X) bytes, so the library will clobber memory > the user did not intend. declare "char buf[sizeof(FILE)]; FILE *x = buf;" on > the stack and uClibc will overwrite random stuff on the stack. Passing this FILE * to stdio functions invokes undefined behavior. They all require a FILE * obtained from fopen/fdopen/popen/etc. or from one of the standard stream pointers (stdin/stdout/stderr). The C standard is very explicit that you are not allowed to use copies of FILEs, and without copying an existing one, there's no way to construct one that would be valid anyway. > > FILE structures are expected to be allocated by the library > > your sub-thread here indicated that someone other than uClibc was allocating > FILE structures (or i misread). that cannot work w/out symbol versioning. > > if uClibc is the only thing that allocates/uses these structures, then > appending members should be fine. No one but uClibc can allocate them. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 04:24:33PM -0400, Mike Frysinger wrote: > > > >>> #endif /* __UCLIBC_HAS_WCHAR__ */ > > > >>> int __filedes; > > > >>> + int __errno_value; > > > >>> #ifdef __STDIO_BUFFERS > > > >>> unsigned char *__bufstart; /* pointer to buffer */ > > > >>> unsigned char *__bufend;/* pointer to 1 past end of > > > >>> buffer */ > > > > > > -> unsigned char *__bufpos; > > > > Indeed, I missed this. It should definitely not be added near the > > beginning of the structure, only past elements that could be part of > > the macro ABI. > > that would help only if the uClibc code itself had versioned functions which > handled the growing struct. if you're attempting to support code that No, there is no need for versioned functions. The uClibc code is in uClibc and will always be using the version of FILE corresponding to itself. Code in other libraries or applications only cares that FILE * is a pointer (it might as well be void *) and about the offsets/types of the buffer pointers (if getc/putc macros are enabled). > allocated sizeof(FILE) memory (which is dumb), then location in the struct > wouldn't matter. There's nothing useful/meaningful that such code could do. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
>>> alarm(1); >>> fgets(buf, sizeof buf, f); > > I was assuming the old standard idiom of installing a do-nothing > signal handler (without SA_RESTART) for SIGALRM so that fgets would > return with an error and the program could proceed. Ugh. I get it. In this case, then, *of course* you want to get EINTR reported to the user. And I already agreed that the libc should not safe-wrap system calls, so yes, naturally fgets should be interrupted. But honestly, this seems to me like the lazy (in a bad sense) way to write such a program; the right way would be to write a little asynchronous select/poll loop with a timeout of 1 second and a reading event on fd 0. The main reason why the latter paradigm isn't used as much is that stdio doesn't handle partial reads and so is quite unusable in asynchronous loops - in other words, stdio sucks. Whenever I need to listen to signals as events (or, in the case of alarm(), as timeouts), I use a self-pipe and a select loop so I can listen to any number of events at the same time. It's a bit harder to program, but much more robust; the "blocking system calls + EINTR" paradigm is ad-hoc and error-prone. It can break so easily, for instance when your libc is misguided and safewraps around EINTR. ;) -- Laurent ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: tar-1.26 not unpacking archives with symlinks?
On Thu, Mar 1, 2012 at 5:45 PM, Bernhard Reutner-Fischer
wrote:
> On Feb 29, 2012 6:33 PM, "Ed W" wrote:
>>
>> Hi, I'm hitting an error when trying to unpack archives with the recently
> unmasked tar-1.26. Basically any symlinks in the archive trigger an
> "unknown file" error. This prevents me using emerge -k amongst other things
>>
>> I only see this on uclibc-0.9.33 based system, not on the build host
> which uses glibc amd64
>>
>> Can anyone else repro this and shed any light on what the problem might
> be? Problem in tar or uclibc?
>>
>
> Can you provide a tarbalk with one file that exhibits the problem, plus an
> strafe of the attempt to unpack it?
I have seen this too and it only happens when the symlink is extracted
before the file it points to.
To reproduce:
touch myfile
ln -s myfile mylink
tar -cf testcase1.tar mylink myfile
If the file comes first (tar -cf testcase2.tar myfile mylink) it will work.
The eopen("../testcase.tar", O_RDONLY|O_LARGEFILE) = 3
read(3, "mylink\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
10240) = 10240
fstat64(3, {st_mode=S_IFREG|0644, st_size=10240, ...}) = 0
fstat64(3, {st_mode=S_IFREG|0644, st_size=10240, ...}) = 0
clock_gettime(CLOCK_REALTIME, {1331674689, 93257}) = 0
open("/etc/passwd", O_RDONLY) = 4
ioctl(4, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or
TCGETS, 0x5e47954c) = -1 ENOTTY (Inappropriate ioctl for device)
brk(0x14e71000) = 0x14e71000
read(4, "root:x:0:0:root:/root:/bin/ash\nb"..., 8192) = 2853
close(4)= 0
open("/etc/group", O_RDONLY)= 4
ioctl(4, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or
TCGETS, 0x5e47954c) = -1 ENOTTY (Inappropriate ioctl for device)
read(4, "root:x:0:root\nbin:x:1:root,bin,d"..., 8192) = 1158
close(4)= 0
symlinkat("myfile", AT_FDCWD, "mylink") = 0
fstatat64(AT_FDCWD, "mylink", {st_mode=S_IFLNK|0777, st_size=6, ...},
AT_SYMLINK_NOFOLLOW) = 0
utimensat(AT_FDCWD, "mylink", {{1331674689, 930453230}, {1331674345,
0}}, AT_SYMLINK_NOFOLLOW) = 0
fstatat64(AT_FDCWD, "mylink", {st_mode=S_IFLNK|0777, st_size=6, ...},
AT_SYMLINK_NOFOLLOW) = 0
fchmodat(AT_FDCWD, "mylink", 0755) = -1 ENOENT (No such file or directory)
write(2, "tar", 3tar) = 3
write(2, ": ", 2: ) = 2
write(2, "mylink", 6mylink) = 6
write(2, ": Cannot change mode to ", 24: Cannot change mode to ) = 24
write(2, "rwxr-xr-x", 9rwxr-xr-x)= 9
write(2, ": ", 2: ) = 2
write(2, "No such file or directory", 25No such file or directory) = 25
write(2, "\n", 1
) = 1
openat(AT_FDCWD, "myfile",
O_WRONLY|O_CREAT|O_EXCL|O_NOCTTY|O_NONBLOCK|O_LARGEFILE|O_CLOEXEC,
0644) = 4
dup2(4, 4) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
utimensat(4, NULL, {{1331674689, 933786545}, {1331674327, 0}}, 0) = 0
close(4)= 0
clock_gettime(CLOCK_REALTIME, {1331674689, 938422399}) = 0
clock_gettime(CLOCK_REALTIME, {1331674689, 938497339}) = 0
close(3)= 0
write(2, "tar", 3tar) = 3
write(2, ": ", 2: ) = 2
write(2, "Exiting with failure status due "..., 50Exiting with failure
status due to previous errors) = 50
write(2, "\n", 1
) = 1
close(1)= 0
close(2)= 0
exit_group(2) = ?
rror message from tar:
tar: mylink: Cannot change mode to rwxr-xr-x: No such file or directory
tar: Exiting with failure status due to previous errors
It's the fchmodat() that fails which makes me wonder why it works on GNU libc.
--
Natanael Copa
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tuesday 13 March 2012 16:56:13 u-uclibc-q...@aetey.se wrote: > On Tue, Mar 13, 2012 at 04:24:33PM -0400, Mike Frysinger wrote: > > > It should definitely not be added near the > > > beginning of the structure, only past elements that could be part of > > > the macro ABI. > > > > that would help only if the uClibc code itself had versioned functions > > which handled the growing struct. > > Curious, why wouldn't this work without versioned functions? if end user code encodes sizeof(FILE) anywhere, they're creating a memory region of a fixed number of bytes, so increasing the structure means that the app will give us a pointer to memory of X bytes, but the library requires it to be Y (where Y is greater than X) bytes, so the library will clobber memory the user did not intend. declare "char buf[sizeof(FILE)]; FILE *x = buf;" on the stack and uClibc will overwrite random stuff on the stack. > FILE structures are expected to be allocated by the library your sub-thread here indicated that someone other than uClibc was allocating FILE structures (or i misread). that cannot work w/out symbol versioning. if uClibc is the only thing that allocates/uses these structures, then appending members should be fine. > Of course this generally only works if the macros are not being changed. correct, any existing members which are exposed via macros as part of the public API would have to remain in place. > > if you're attempting to support code that > > allocated sizeof(FILE) memory (which is dumb), then location in the > > struct wouldn't matter. > > I don't think keeping the size of FILE constant is practically important, > even though I mentioned this as an incompatibility. if by "practically" you mean we don't care about the end users who are relying on sizeof(FILE), then sure (i don't care about that use case). i was just being pedantic in my reply. -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 04:24:33PM -0400, Mike Frysinger wrote: > > It should definitely not be added near the > > beginning of the structure, only past elements that could be part of > > the macro ABI. > > that would help only if the uClibc code itself had versioned functions which > handled the growing struct. Curious, why wouldn't this work without versioned functions? FILE structures are expected to be allocated by the library and used by 1. the library and 2. the macros. If we just add extra items at the end, neither the new library code would be confused (it knows about the new size and contents), nor the old application code would be confused (the macros use[d] the offsets which did not change, the calls to the functions pass pointers not structures). Of course this generally only works if the macros are not being changed. > if you're attempting to support code that > allocated sizeof(FILE) memory (which is dumb), then location in the struct > wouldn't matter. I don't think keeping the size of FILE constant is practically important, even though I mentioned this as an incompatibility. Anyway, everyone now seems to agree that the proposed patch breaks the ABI :) > -mike Regards, Rune ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tuesday 13 March 2012 15:32:01 Rich Felker wrote: > On Tue, Mar 13, 2012 at 07:19:35PM +, u-uclibc-q...@aetey.se wrote: > > > there's no correct/sane > > > program that would be affected by this ABI change. > > > > Actually I suspect that stdio macros might use stuff (like __bufpos) > > which is located past the proposed new structure member, in which case > > existing binaries would become incompatible with a patched library: > > > > >>> #endif /* __UCLIBC_HAS_WCHAR__ */ > > >>> int __filedes; > > >>> + int __errno_value; > > >>> #ifdef __STDIO_BUFFERS > > >>> unsigned char *__bufstart; /* pointer to buffer */ > > >>> unsigned char *__bufend;/* pointer to 1 past end of > > >>> buffer */ > > > > -> unsigned char *__bufpos; > > Indeed, I missed this. It should definitely not be added near the > beginning of the structure, only past elements that could be part of > the macro ABI. that would help only if the uClibc code itself had versioned functions which handled the growing struct. if you're attempting to support code that allocated sizeof(FILE) memory (which is dumb), then location in the struct wouldn't matter. -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tuesday 13 March 2012 01:58:49 Rich Felker wrote:
> On Tue, Mar 13, 2012 at 01:41:01AM -0400, Mike Frysinger wrote:
> > On Sunday 11 March 2012 11:12:19 Denys Vlasenko wrote:
> > > --- a/libc/sysdeps/linux/common/bits/uClibc_stdio.h
> > > +++ b/libc/sysdeps/linux/common/bits/uClibc_stdio.h
> > > @@ -250,6 +250,7 @@ struct __STDIO_FILE_STRUCT {
> > > unsigned char __ungot[2];
> > > #endif /* __UCLIBC_HAS_WCHAR__ */
> > > int __filedes;
> > > + int __errno_value;
> > > #ifdef __STDIO_BUFFERS
> > > unsigned char *__bufstart; /* pointer to buffer */
> > > unsigned char *__bufend;/* pointer to 1 past end of buffer */
> >
> > pretty sure this breaks ABI. i know we aren't completely strict on this,
> > but it's something we should avoid if possible.
>
> How so? Application code should not be touching the internals of
> FILE...
because the uClibc macros which implement the public API directly access the
structure contents. look at the getc macros and how it leverages the locking
members.
-mike
signature.asc
Description: This is a digitally signed message part.
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 07:19:35PM +, u-uclibc-q...@aetey.se wrote: > > there's no correct/sane > > program that would be affected by this ABI change. > > Actually I suspect that stdio macros might use stuff (like __bufpos) > which is located past the proposed new structure member, in which case > existing binaries would become incompatible with a patched library: > > >>> #endif /* __UCLIBC_HAS_WCHAR__ */ > >>> int __filedes; > >>> + int __errno_value; > >>> #ifdef __STDIO_BUFFERS > >>> unsigned char *__bufstart; /* pointer to buffer */ > >>> unsigned char *__bufend;/* pointer to 1 past end of buffer */ > -> unsigned char *__bufpos; Indeed, I missed this. It should definitely not be added near the beginning of the structure, only past elements that could be part of the macro ABI. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 01:14:12PM -0400, Rich Felker wrote: > SIZE_MAX to prevent applications from declaring objects of type > FILE... :-) :-) I guess you are right and nobody declares them. > there's no correct/sane > program that would be affected by this ABI change. Actually I suspect that stdio macros might use stuff (like __bufpos) which is located past the proposed new structure member, in which case existing binaries would become incompatible with a patched library: >>> #endif /* __UCLIBC_HAS_WCHAR__ */ >>> int __filedes; >>> + int __errno_value; >>> #ifdef __STDIO_BUFFERS >>> unsigned char *__bufstart; /* pointer to buffer */ >>> unsigned char *__bufend;/* pointer to 1 past end of buffer */ -> unsigned char *__bufpos; Rune ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 06:59:03PM +0100, Laurent Bercot wrote: > > There is no reason to "protect" reads against EINTR because EINTR > > cannot happen unless the application specifically chooses for it to > > happen (by installing a non-restarting signal handler). EINTR does not > > just randomly happen. > > It can, if you take the standard literally. I agree that you *should* > be able to assume that unblockable signals have SA_RESTART behaviour, > that would simplify Unix programming a whole lot. But as of today, you > can't, and I'd rather be on the safe side. I'd have to go back and look at the (disorganized) language about it in the standard, but you may be right about the lack of a strict requirement. I would consider this a flaw in the standard since it largely prevents using EINTR in any useful way. There's a similar bug in the statement of pthread cancellation behavior that makes it so implementations are allowed (and glibc/uClibc exercise this allowance!) to leak resources or worse (corrupt fd state) randomly when cancellation is performed; allowing implementations of such low quality essentially makes the feature unusable. This should really all be documented well and submitted as defect reports... > > My claim is that code doing such protection is wrong and harmful to > > begin with. > > I disagree, because it has its uses. But I think we can agree that it > is wrong to do it *in the libc*. The programmer should have the choice > to handle EINTR himself. In some places in a program you might know it's essential to complete the operation and not lose data, in which case you're right, but this sort of situation will require specific contracts between the callee and caller about what to do. In general, I feel like the appropriate response to EINTR in the absence of a contract with the caller is to assume that when the caller has arranged for EINTR to happen, the caller wants you to stop when interrupted. > > Think of programs that do something like: > > alarm(1); > > fgets(buf, sizeof buf, f); > > SIGALRM is raised. SIGALRM isn't caught. SIGALRM kills the process, > even if it was blocking on a safe_read() function. Where's the problem? I was assuming the old standard idiom of installing a do-nothing signal handler (without SA_RESTART) for SIGALRM so that fgets would return with an error and the program could proceed. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
> There is no reason to "protect" reads against EINTR because EINTR > cannot happen unless the application specifically chooses for it to > happen (by installing a non-restarting signal handler). EINTR does not > just randomly happen. It can, if you take the standard literally. I agree that you *should* be able to assume that unblockable signals have SA_RESTART behaviour, that would simplify Unix programming a whole lot. But as of today, you can't, and I'd rather be on the safe side. > My claim is that code doing such protection is wrong and harmful to > begin with. I disagree, because it has its uses. But I think we can agree that it is wrong to do it *in the libc*. The programmer should have the choice to handle EINTR himself. > Think of programs that do something like: > alarm(1); > fgets(buf, sizeof buf, f); SIGALRM is raised. SIGALRM isn't caught. SIGALRM kills the process, even if it was blocking on a safe_read() function. Where's the problem? -- Laurent ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 04:06:35PM +, u-uclibc-q...@aetey.se wrote: > Hi Rich, > > On Tue, Mar 13, 2012 at 10:36:48AM -0400, Rich Felker wrote: > > > > On Tue, Mar 13, 2012 at 01:41:01AM -0400, Mike Frysinger wrote: > > > > > > int __filedes; > > > > > > + int __errno_value; > > > > You are right Rich but it seems you are thinking of API while this change > > > breaks ABI. > > > > If it does, I still don't see how it could. Applications cannot > > declare objects of type FILE, only FILE *. Short of some macros that > > poke at the early members of the structure (getc/putc macros), it's > > Applications actually can declare, allocate, copy FILE objects > unless explicitely prohibited by the specification - does it > prohibit this? All I found is a mention that a copy of a FILE object > is unusable at a different address than the original > (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf page 266 > last paragraph). Applications presumably can do this, but the only use for doing so would be to use the object as a buffer of type `char[sizeof(FILE)]`. It can't be used for any purpose connected with stdio. And being that the size is very arbitrary, I can't see how you could use it for anything useful; the only thing you know about `sizeof(FILE)` is that it's greater than or equal to 1. Presumably it could even have size SIZE_MAX to prevent applications from declaring objects of type FILE... :-) > That's why I assume that the FILE structure as a whole is a part of the > ABI, despite being opaque. > > I may be wrong reading/interpreting specification(s), then please > correct me. > > The actual change being discussed may be compatible in practice, > I take your word for this! I believe you may be technically correct, but there's no correct/sane program that would be affected by this ABI change. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
Hi Rich, On Tue, Mar 13, 2012 at 10:36:48AM -0400, Rich Felker wrote: > > > On Tue, Mar 13, 2012 at 01:41:01AM -0400, Mike Frysinger wrote: > > > > > int __filedes; > > > > > + int __errno_value; > > You are right Rich but it seems you are thinking of API while this change > > breaks ABI. > > If it does, I still don't see how it could. Applications cannot > declare objects of type FILE, only FILE *. Short of some macros that > poke at the early members of the structure (getc/putc macros), it's Applications actually can declare, allocate, copy FILE objects unless explicitely prohibited by the specification - does it prohibit this? All I found is a mention that a copy of a FILE object is unusable at a different address than the original (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf page 266 last paragraph). I can not think of an apparent meaningful reason for copying FILE or using sizeof(FILE) but this by itself doesn't mean there isn't any. (sorry for a phrase with three negations...) That's why I assume that the FILE structure as a whole is a part of the ABI, despite being opaque. I may be wrong reading/interpreting specification(s), then please correct me. The actual change being discussed may be compatible in practice, I take your word for this! > Rich Regards, Rune ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 01:46:39AM -0400, Mike Frysinger wrote: > treating EINTR/EAGAIN as unrecoverable is bad behavior for an app. there are > valid ways an app could receive either signal and have it be normal behavior. Receiving a signal does not result in EINTR. Running a signal handler that was installed (intentionally) without SA_RESTART is what causes EINTR. > having that handling be done in uClibc is not required by POSIX, but it's > better imo for uClibc to carry a little bit more code there to provide better > reliability to higher layers rather than forcing every userspace app to > implement their own retry-on-eagain macros. Retrying on EAGAIN is simply *wrong*. It makes you consume 100% cpu. If the file descriptor is set as EAGAIN, it means whoever set it up wants to ensure that blocking does not happen; emulating blocking with 100% cpu load is the worst possible way to handle this situation! For stdio, EAGAIN must be treated as a hard error. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 10:30:10AM +0100, Laurent Bercot wrote: > > Note that I'm claiming the whole "safe_read"/"safe_write" idiom is > > wrong, and a throwback to the pre-sigaction SysV era when signal() was > > the only way to install a signal handler and it gave you > > non-restarting semantics. The idea of an interrupting signal, when set > > intentionally by a modern application using sigaction, is that EINTR > > should be treated as a (usually permanent/unrecoverable) failure on > > blocked in-progress IO operations. > > Hi Rich, > > I'm not pronouncing on stdio, since stdio doesn't really mix with > asynchronous event loops anyway; but in an event-driven program that > reacts to signals as well as fd events or timeouts, signals can > arrive at any time and should be handled by the application at the > same level as any other event, in a normal context, *not* right when > they arrive in an interrupt context. (Think pselect(), or the self-pipe > trick.) I agree that interrupting signals are not very useful for an event-driven application. This is not their intended usage case. > In this case, it is absolutely necessary to protect every read() and > write() operation, as well as any interruptible system call, against > EINTR, because EINTR is not reporting a failure, it is only reporting > the arrival of a signal at an inconvenient time. EINTR is basically reporting that the user hit Ctrl+C and the application wants the library routine that was in progress to return with an error rather than asynchronously exiting, probably because it has unwritten state that can't be accessed asynchronously for saving. There is no reason to "protect" reads against EINTR because EINTR cannot happen unless the application specifically chooses for it to happen (by installing a non-restarting signal handler). EINTR does not just randomly happen. > This protection can be done at the user level, but there is nothing wrong My claim is that code doing such protection is wrong and harmful to begin with. > Why not simply let the user set SA_RESTART in the sigaction() call? > see http://www.skarnet.org/software/skalibs/libstddjb/safewrappers.html I know of no modern system where this kind of bogus EINTR happens (SIGSTOP, etc.). Even the proprietary unices fixed it before Linux did, but they've all been fixed for a long time now. I agree the standard should be more explicit on this; failure to be explicit, if you interpret it literally, makes interrupting signals unusable since you have to wrap all your syscalls with loops and make them behave as if the signal was a restarting one.. I think the difference in opinion about EINTR comes from two different ideological camps: one which believes the "Worse is Better" story that the very existence of EINTR was a bug in early Unix systems, and another which believes EINTR was/is a feature to allow a program interrupted during a blocked operation by a (usually user-generated) signal to terminate or return to an initial state cleanly without breaking the rules of async signal safety, discarding whatever intermediate work it was doing. Think of programs that do something like: alarm(1); fgets(buf, sizeof buf, f); Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 07:16:54AM +, u-uclibc-q...@aetey.se wrote: > On Tue, Mar 13, 2012 at 01:58:49AM -0400, Rich Felker wrote: > > On Tue, Mar 13, 2012 at 01:41:01AM -0400, Mike Frysinger wrote: > > > > int __filedes; > > > > + int __errno_value; > > > > pretty sure this breaks ABI. i know we aren't completely strict on this, > > > but > > > it's something we should avoid if possible. > > > > How so? Application code should not be touching the internals of > > FILE... > > You are right Rich but it seems you are thinking of API while this change > breaks ABI. If it does, I still don't see how it could. Applications cannot declare objects of type FILE, only FILE *. Short of some macros that poke at the early members of the structure (getc/putc macros), it's purely an opaque type, so changes to its size or layout (beyond said early members) should not break ABI. If you claim it does, please explain how you think it does, because the rest of us are missing that issue.. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Sunday 11 March 2012 23:36:58 Rich Felker wrote: > On Mon, Mar 12, 2012 at 01:50:27AM +0100, Denys Vlasenko wrote: > > On Sunday 11 March 2012 18:10, Rich Felker wrote: > > > On Sun, Mar 11, 2012 at 04:12:19PM +0100, Denys Vlasenko wrote: > > > > On Sunday 11 March 2012 14:46, Denys Vlasenko wrote: > > > > > I propose the following patch. > > > > > Only compile tested by me, care to run-test? > > > > > > > > A more correct (I hope) version: > > > I'm curious why this is such a big patch introducing new functions and > > > struct members. Couldn't it be just a one-line change in the function > > > that writes out the buffers (i.e. do the buffer-voiding as soon as the > > > error is hit rather than testing for it later)? > > > > EINTR and EAGAIN should not result in output buffers being dropped. > > You need to remember the error code for this. > > Why? I don't see any way an application could make use of this > behavior to avoid losing data on EINTR or EAGAIN. As an application > writer I would just treat any write error (or short write) from stdio > as an unrecoverable situation. Even if some implementations might find > a way to make it so an application could recover (I'd be interested in > hearing if you have an idea how this would work...), I don't see any > part of the standard that requires the implementation to behave that > way, so an application depending on it would be non-portable. treating EINTR/EAGAIN as unrecoverable is bad behavior for an app. there are valid ways an app could receive either signal and have it be normal behavior. having that handling be done in uClibc is not required by POSIX, but it's better imo for uClibc to carry a little bit more code there to provide better reliability to higher layers rather than forcing every userspace app to implement their own retry-on-eagain macros. -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
> Note that I'm claiming the whole "safe_read"/"safe_write" idiom is > wrong, and a throwback to the pre-sigaction SysV era when signal() was > the only way to install a signal handler and it gave you > non-restarting semantics. The idea of an interrupting signal, when set > intentionally by a modern application using sigaction, is that EINTR > should be treated as a (usually permanent/unrecoverable) failure on > blocked in-progress IO operations. Hi Rich, I'm not pronouncing on stdio, since stdio doesn't really mix with asynchronous event loops anyway; but in an event-driven program that reacts to signals as well as fd events or timeouts, signals can arrive at any time and should be handled by the application at the same level as any other event, in a normal context, *not* right when they arrive in an interrupt context. (Think pselect(), or the self-pipe trick.) In this case, it is absolutely necessary to protect every read() and write() operation, as well as any interruptible system call, against EINTR, because EINTR is not reporting a failure, it is only reporting the arrival of a signal at an inconvenient time. This protection can be done at the user level, but there is nothing wrong with safe_read and safe_write functions. My skalibs library provides safe wrappers for most interruptible system calls, so the program flow doesn't get disturbed by incoming signals; signals will get handled in their own time anyway. Why not simply let the user set SA_RESTART in the sigaction() call? see http://www.skarnet.org/software/skalibs/libstddjb/safewrappers.html You are right about EAGAIN though. The point of EAGAIN is for a non-blocking system call to return immediately instead of blocking, and it should never be hidden to the user. -- Laurent ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] prevent retries on fclose/fflush after write errors
On Tue, Mar 13, 2012 at 01:58:49AM -0400, Rich Felker wrote: > On Tue, Mar 13, 2012 at 01:41:01AM -0400, Mike Frysinger wrote: > > > int __filedes; > > > + int __errno_value; > > pretty sure this breaks ABI. i know we aren't completely strict on this, > > but > > it's something we should avoid if possible. > > How so? Application code should not be touching the internals of > FILE... You are right Rich but it seems you are thinking of API while this change breaks ABI. Cheers, Rune ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
