Cleaned up the logging command-line option variables:
- renamed VG_(logging_to_filedes) as VG_(logging_to_socket), which is
clearer
- no longer exporting clo_log_to, which avoids the confusion about whether
it or VG_(logging_to_socket) actually controls where output goes
- couple of other minor cleanups
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@3720 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/core.h b/coregrind/core.h
index 05049fc..f10abe3 100644
--- a/coregrind/core.h
+++ b/coregrind/core.h
@@ -151,15 +151,6 @@
/* The max number of suppression files. */
#define VG_CLO_MAX_SFILES 10
-/* Describes where logging output is to be sent. */
-typedef
- enum {
- VgLogTo_Fd,
- VgLogTo_File,
- VgLogTo_FileExactly,
- VgLogTo_Socket
- } VgLogTo;
-
/* Application-visible file descriptor limits */
extern Int VG_(fd_soft_limit);
extern Int VG_(fd_hard_limit);
@@ -184,22 +175,22 @@
/* Where logging output is to be sent to.
- When log_to == VgLogTo_Fd, clo_log_fd holds the file id, and is
- taken from the command line. clo_log_name is irrelevant.
+ With --log-fd (and by default), clo_log_fd holds the file id, and is
+ taken from the command line. (fd 2, stderr, is the default.)
+ clo_log_name is irrelevant.
- When log_to == VgLogTo_File, clo_log_name holds the log-file
+ With --log-file/--log-file-exactly, clo_log_name holds the log-file
name, and is taken from the command line. clo_log_fd is then
made to hold the relevant file id, by opening clo_log_name
(concatenated with the process ID) for writing.
- When log_to == VgLogTo_Socket, clo_log_name holds the
- hostname:portnumber pair, and is taken from the command line.
- clo_log_fd is then made to hold the relevant file handle, by
- opening a connection to said hostname:portnumber pair.
+ With --log-socket, clo_log_name holds the hostname:portnumber pair,
+ and is taken from the command line. clo_log_fd is then made to hold
+ the relevant file handle, by opening a connection to that
+ hostname:portnumber pair.
Global default is to set log_to == VgLogTo_Fd and log_fd == 2
(stderr). */
-extern VgLogTo VG_(clo_log_to);
extern Int VG_(clo_log_fd);
extern Char* VG_(clo_log_name);
@@ -698,7 +689,7 @@
/* Tell the logging mechanism whether we are logging to a file
descriptor or a socket descriptor. */
-extern Bool VG_(logging_to_filedes);
+extern Bool VG_(logging_to_socket);
/* Sanity checks which may be done at any time. The scheduler decides when. */
extern void VG_(sanity_check_general) ( Bool force_expensive );
diff --git a/coregrind/vg_main.c b/coregrind/vg_main.c
index 72e7a79..a18a69f 100644
--- a/coregrind/vg_main.c
+++ b/coregrind/vg_main.c
@@ -164,7 +164,7 @@
/* Tell the logging mechanism whether we are logging to a file
descriptor or a socket descriptor. */
-Bool VG_(logging_to_filedes) = True;
+Bool VG_(logging_to_socket) = False;
/*====================================================================*/
@@ -1256,15 +1256,15 @@
/*=== Command line errors ===*/
/*====================================================================*/
-static void abort_msg ( void )
+static void revert_to_stderr ( void )
{
- VG_(clo_log_to) = VgLogTo_Fd;
+ vg_assert( !VG_(logging_to_socket) );
VG_(clo_log_fd) = 2; /* stderr */
}
void VG_(bad_option) ( Char* opt )
{
- abort_msg();
+ revert_to_stderr();
VG_(printf)("valgrind: Bad option `%s'; aborting.\n", opt);
VG_(printf)("valgrind: Use --help for more information.\n");
VG_(exit)(1);
@@ -1272,7 +1272,7 @@
static void missing_prog ( void )
{
- abort_msg();
+ revert_to_stderr();
VG_(printf)("valgrind: no program specified\n");
VG_(printf)("valgrind: Use --help for more information.\n");
VG_(exit)(1);
@@ -1280,7 +1280,7 @@
static void config_error ( Char* msg )
{
- abort_msg();
+ revert_to_stderr();
VG_(printf)("valgrind: Startup or configuration error:\n %s\n", msg);
VG_(printf)("valgrind: Unable to start up properly. Giving up.\n");
VG_(exit)(1);
@@ -1410,11 +1410,8 @@
Bool VG_(clo_demangle) = True;
Bool VG_(clo_trace_children) = False;
-/* See big comment in core.h for meaning of these three.
- fd is initially stdout, for --help, but gets moved to stderr by default
- immediately afterwards. */
-VgLogTo VG_(clo_log_to) = VgLogTo_Fd;
-Int VG_(clo_log_fd) = 1;
+/* See big comment in core.h for meaning of these two. */
+Int VG_(clo_log_fd) = 2;
Char* VG_(clo_log_name) = NULL;
Bool VG_(clo_time_stamp) = False;
@@ -1546,6 +1543,10 @@
" tool's start-up message for more information.\n"
"\n";
+ // Ensure the message goes to stdout
+ VG_(clo_log_fd) = 1;
+ vg_assert( !VG_(logging_to_socket) );
+
VG_(printf)(usage1);
if (VG_(details).name) {
VG_(printf)(" user options for %s:\n", VG_(details).name);
@@ -1604,6 +1605,12 @@
{
Int i, eventually_log_fd;
Int toolname_len = VG_(strlen)(toolname);
+ enum {
+ VgLogTo_Fd,
+ VgLogTo_File,
+ VgLogTo_FileExactly,
+ VgLogTo_Socket
+ } log_to = VgLogTo_Fd; // Where is logging output to be sent?
/* log to stderr by default, but usage message goes to stdout */
eventually_log_fd = 2;
@@ -1721,23 +1728,23 @@
VG_(clo_vex_control).guest_chase_thresh, 0, 99)
else if (VG_CLO_STREQN(9, arg, "--log-fd=")) {
- VG_(clo_log_to) = VgLogTo_Fd;
+ log_to = VgLogTo_Fd;
VG_(clo_log_name) = NULL;
eventually_log_fd = (Int)VG_(atoll)(&arg[9]);
}
else if (VG_CLO_STREQN(11, arg, "--log-file=")) {
- VG_(clo_log_to) = VgLogTo_File;
+ log_to = VgLogTo_File;
VG_(clo_log_name) = &arg[11];
}
else if (VG_CLO_STREQN(19, arg, "--log-file-exactly=")) {
- VG_(clo_log_to) = VgLogTo_FileExactly;
+ log_to = VgLogTo_FileExactly;
VG_(clo_log_name) = &arg[19];
}
else if (VG_CLO_STREQN(13, arg, "--log-socket=")) {
- VG_(clo_log_to) = VgLogTo_Socket;
+ log_to = VgLogTo_Socket;
VG_(clo_log_name) = &arg[13];
}
@@ -1836,18 +1843,31 @@
VG_(bad_option)("--db-attach=yes and --trace-children=yes");
}
- /* Set up logging now. After this is done, VG_(clo_log_fd)
+ if (VG_(clo_gen_suppressions) > 0 &&
+ !VG_(needs).core_errors && !VG_(needs).tool_errors) {
+ VG_(message)(Vg_UserMsg,
+ "Can't use --gen-suppressions= with this tool,");
+ VG_(message)(Vg_UserMsg,
+ "as it doesn't generate errors.");
+ VG_(bad_option)("--gen-suppressions=");
+ }
+
+ /* All non-logging-related options have been checked. If the logging
+ option specified is ok, we can switch to it, as we know we won't
+ have to generate any other command-line-related error messages.
+ (So far we should be still attached to stderr, so we can show on
+ the terminal any problems to do with processing command line
+ opts.)
+
+ So set up logging now. After this is done, VG_(clo_log_fd)
should be connected to whatever sink has been selected, and we
indiscriminately chuck stuff into it without worrying what the
nature of it is. Oh the wonder of Unix streams. */
- /* So far we should be still attached to stdout, so we can show on
- the terminal any problems to do with processing command line
- opts. */
- vg_assert(VG_(clo_log_fd) == 1 /* stdout */);
- vg_assert(VG_(logging_to_filedes) == True);
+ vg_assert(VG_(clo_log_fd) == 2 /* stderr */);
+ vg_assert(VG_(logging_to_socket) == False);
- switch (VG_(clo_log_to)) {
+ switch (log_to) {
case VgLogTo_Fd:
vg_assert(VG_(clo_log_name) == NULL);
@@ -1871,6 +1891,7 @@
VG_(clo_log_name), pid, seq );
seq++;
+ // EXCL: it will fail with EEXIST if the file already exists.
eventually_log_fd
= VG_(open)(logfilename,
VKI_O_CREAT|VKI_O_WRONLY|VKI_O_EXCL|VKI_O_TRUNC,
@@ -1879,13 +1900,15 @@
VG_(clo_log_fd) = VG_(safe_fd)(eventually_log_fd);
break; /* for (;;) */
} else {
+ // If the file already existed, we try the next name. If it
+ // was some other file error, we give up.
if (eventually_log_fd != -VKI_EEXIST) {
VG_(message)(Vg_UserMsg,
"Can't create/open log file `%s.pid%d'; giving up!",
VG_(clo_log_name), pid);
VG_(bad_option)(
"--log-file=<file> (didn't work out for some reason.)");
- break; /* for (;;) */
+ /*NOTREACHED*/
}
}
}
@@ -1893,20 +1916,16 @@
}
case VgLogTo_FileExactly: {
- Char logfilename[1000];
-
vg_assert(VG_(clo_log_name) != NULL);
vg_assert(VG_(strlen)(VG_(clo_log_name)) <= 900); /* paranoia */
- VG_(sprintf)(logfilename, "%s", VG_(clo_log_name));
eventually_log_fd
- = VG_(open)(logfilename,
+ = VG_(open)(VG_(clo_log_name),
VKI_O_CREAT|VKI_O_WRONLY|VKI_O_TRUNC,
VKI_S_IRUSR|VKI_S_IWUSR);
if (eventually_log_fd >= 0) {
VG_(clo_log_fd) = VG_(safe_fd)(eventually_log_fd);
- }
- else if (eventually_log_fd != -VKI_EEXIST) {
+ } else {
VG_(message)(Vg_UserMsg,
"Can't create/open log file `%s'; giving up!",
VG_(clo_log_name));
@@ -1928,6 +1947,7 @@
"of `%s'; giving up!", VG_(clo_log_name) );
VG_(bad_option)(
"--log-socket=");
+ /*NOTREACHED*/
}
if (eventually_log_fd == -2) {
VG_(message)(Vg_UserMsg,
@@ -1938,17 +1958,21 @@
VG_(message)(Vg_UserMsg,
"" );
/* We don't change anything here. */
+ vg_assert(VG_(clo_log_fd) == 2);
} else {
vg_assert(eventually_log_fd > 0);
VG_(clo_log_fd) = eventually_log_fd;
- VG_(logging_to_filedes) = False;
+ VG_(logging_to_socket) = True;
}
break;
}
-
}
- /* Move log_fd into the safe range, so it doesn't conflict with any app fds */
+ // Move log_fd into the safe range, so it doesn't conflict with any app fds.
+ // XXX: this is more or less duplicating the behaviour of the calls to
+ // VG_(safe_fd)() above, although this does not close the original fd.
+ // Perhaps the VG_(safe_fd)() calls above should be removed, and this
+ // code should be replaced with a call to VG_(safe_fd)(). --njn
eventually_log_fd = VG_(fcntl)(VG_(clo_log_fd), VKI_F_DUPFD, VG_(fd_hard_limit));
if (eventually_log_fd < 0)
VG_(message)(Vg_UserMsg, "valgrind: failed to move logfile fd into safe range");
@@ -1985,7 +2009,7 @@
"Copyright (C) 2000-2005, and GNU GPL'd, by Julian Seward et al.");
}
- if (VG_(clo_verbosity) > 0 && VG_(clo_log_to) != VgLogTo_Fd) {
+ if (VG_(clo_verbosity) > 0 && log_to != VgLogTo_Fd) {
VG_(message)(Vg_UserMsg, "");
VG_(message)(Vg_UserMsg,
"My PID = %d, parent PID = %d. Prog and args are:",
@@ -1996,7 +2020,7 @@
if (VG_(clo_verbosity) > 1) {
Int fd;
- if (VG_(clo_log_to) != VgLogTo_Fd)
+ if (log_to != VgLogTo_Fd)
VG_(message)(Vg_DebugMsg, "");
VG_(message)(Vg_DebugMsg, "Valgrind library directory: %s", VG_(libdir));
VG_(message)(Vg_DebugMsg, "Command line");
@@ -2016,7 +2040,7 @@
#define BUF_LEN 256
Char version_buf[BUF_LEN];
Int n = VG_(read) ( fd, version_buf, BUF_LEN );
- vg_assert(n <= 256);
+ vg_assert(n <= BUF_LEN);
if (n > 0) {
version_buf[n-1] = '\0';
VG_(message)(Vg_DebugMsg, " %s", version_buf);
@@ -2039,15 +2063,6 @@
VG_(clo_suppressions)[VG_(clo_n_suppressions)] = buf;
VG_(clo_n_suppressions)++;
}
-
- if (VG_(clo_gen_suppressions) > 0 &&
- !VG_(needs).core_errors && !VG_(needs).tool_errors) {
- VG_(message)(Vg_UserMsg,
- "Can't use --gen-suppressions= with this tool,");
- VG_(message)(Vg_UserMsg,
- "as it doesn't generate errors.");
- VG_(bad_option)("--gen-suppressions=");
- }
}
// Build the string for VALGRINDCLO.
diff --git a/coregrind/vg_mylibc.c b/coregrind/vg_mylibc.c
index 9259a6b..d6d70a0 100644
--- a/coregrind/vg_mylibc.c
+++ b/coregrind/vg_mylibc.c
@@ -395,14 +395,13 @@
/* Do the low-level send of a message to the logging sink. */
static void send_bytes_to_logging_sink ( Char* msg, Int nbytes )
{
- if (VG_(logging_to_filedes)) {
+ if (!VG_(logging_to_socket)) {
VG_(write)( VG_(clo_log_fd), msg, nbytes );
} else {
Int rc = VG_(write_socket)( VG_(clo_log_fd), msg, nbytes );
if (rc == -1) {
// For example, the listener process died. Switch back to stderr.
- VG_(logging_to_filedes) = True;
- VG_(clo_log_to) = VgLogTo_Fd;
+ VG_(logging_to_socket) = False;
VG_(clo_log_fd) = 2;
VG_(write)( VG_(clo_log_fd), msg, nbytes );
}