|
From: <sv...@va...> - 2005-10-04 22:27:26
|
Author: sewardj
Date: 2005-10-04 23:27:22 +0100 (Tue, 04 Oct 2005)
New Revision: 4860
Log:
Rewrite ML_(fd_allowed):
* include explaination from Tom
* make logic easier to follow, and add comments
* remove veto on the -d file descriptor (detailed comments in code)
Modified:
trunk/coregrind/m_syswrap/syswrap-generic.c
Modified: trunk/coregrind/m_syswrap/syswrap-generic.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/coregrind/m_syswrap/syswrap-generic.c 2005-10-04 20:00:20 UTC (=
rev 4859)
+++ trunk/coregrind/m_syswrap/syswrap-generic.c 2005-10-04 22:27:22 UTC (=
rev 4860)
@@ -995,12 +995,65 @@
/* ---------------------------------------------------------------------
Vet file descriptors for sanity
------------------------------------------------------------------ */
+/*=20
+> - what does the "Bool soft" parameter mean?
=20
+(Tom Hughes, 3 Oct 05):
+
+Whether or not to consider a file descriptor invalid if it is above
+the current soft limit.
+
+Basically if we are testing whether a newly created file descriptor is
+valid (in a post handler) then we set soft to true, and if we are
+testing whether a file descriptor that is about to be used (in a pre
+handler) is valid [viz, an already-existing fd] then we set it to false.
+
+The point is that if the (virtual) soft limit is lowered then any
+existing descriptors can still be read/written/closed etc (so long as
+they are below the valgrind reserved descriptors) but no new
+descriptors can be created above the new soft limit.
+
+(jrs 4 Oct 05: in which case, I've renamed it "isNewFd")
+*/
+
/* Return true if we're allowed to use or create this fd */
-Bool ML_(fd_allowed)(Int fd, const Char *syscallname, ThreadId tid, Bool=
soft)
+Bool ML_(fd_allowed)(Int fd, const Char *syscallname, ThreadId tid, Bool=
isNewFd)
{
- if ( (fd < 0 || fd >=3D VG_(fd_hard_limit) || fd =3D=3D VG_(clo_log_f=
d))=20
- && VG_(showing_core_errors)() ) {
+ Bool allowed =3D True;
+
+ /* hard limits always apply */
+ if (fd < 0 || fd >=3D VG_(fd_hard_limit))
+ allowed =3D False;
+
+ /* hijacking the logging fd is never allowed */
+ if (fd =3D=3D VG_(clo_log_fd))
+ allowed =3D False;
+
+ /* if creating a new fd (rather than using an existing one), the
+ soft limit must also be observed */
+ if (isNewFd && fd >=3D VG_(fd_soft_limit))
+ allowed =3D False;
+
+ /* this looks like it ought to be included, but causes problems: */
+ /*
+ if (fd =3D=3D 2 && VG_(debugLog_getLevel)() > 0)
+ allowed =3D False;
+ */
+ /* The difficulty is as follows: consider a program P which expects
+ to be able to mess with (redirect) its own stderr (fd 2).
+ Usually to deal with P we would issue command line flags to send
+ logging somewhere other than stderr, so as not to disrupt P.
+ The problem is that -d unilaterally hijacks stderr with no
+ consultation with P. And so, if this check is enabled, P will
+ work OK normally but fail if -d is issued.
+
+ Basically -d is a hack and you take your chances when using it.
+ It's very useful for low level debugging -- particularly at
+ startup -- and having its presence change the behaviour of the
+ client is exactly what we don't want. */
+
+ /* croak? */
+ if ((!allowed) && VG_(showing_core_errors)() ) {
VG_(message)(Vg_UserMsg,=20
"Warning: invalid file descriptor %d in syscall %s()",
fd, syscallname);
@@ -1010,19 +1063,9 @@
if (VG_(clo_verbosity) > 1) {
VG_(get_and_pp_StackTrace)(tid, VG_(clo_backtrace_size));
}
- return False;
}
- else=20
- if (soft && fd >=3D VG_(fd_soft_limit)) {
- return False;
- }
- else=20
- if (fd =3D=3D 2 && VG_(debugLog_getLevel)() > 0) {
- return False;
- }=20
- else {
- return True;
- }
+
+ return allowed;
}
=20
=20
|