Take 3 attached, altered some, slightly altered log at end.
Martin Sebor wrote:
Andrew Black wrote:
Revised patch, attached. I believe the same change log can be used.
[...]
One version of the patch did do a case insensitive comparison, moving
the rw_strcasecmp (and rw_charcasecmp) functions from exec.cpp to
util.cpp. The reason I chose to do things this way was so that
different warning parsing modes could be specified for the compile and
link stages. An example where this could be useful would be when
building with GCC on Solaris, but linking with Sun's linker.
That's a good point. But I'm afraid I'm not sure I see how
capitalization can help with which linker is used.
I was using the same capitalization trick that was used for setting
hard/soft limits. I've reworked the parse_warn_opts method, simplifying
the logic and accepting only a single alias code (in lower case). A
future patch would likely be to accept case insensitive alias names.
[...]
[...]
This is a math overflow check. If we assume ~5 assertions take 1 k of
disk space in a result log (each assertion ~200 characters (bytes)
long), the resulting file would be ~800,000,000 kb, or ~800 Gb. A
file this long is theoretically within the capabilities of a modern
OS, but you are likely to run out of disk space prior to writing the
entire log.
This check was added for consistency with the assertion counting code,
and it would be outside the scope of this patch to remove that check
(and the ST_OVERFLOW item).
Maybe. It doesn't mean that we need to introduce unnecessary logic
into new code just for consistency with existing code. I don't
suppose you would introduce a bug into new code just because we
had the same bug in a similar piece of existing code, would you? :)
Incidentally, if you insist on handling this exceedingly unlikely
case, it would be better to indicate the overflow in the column
that overflowed (since there are more than one) and not in the
status column.
It makes sense. In this revised patch, I set the value of t_warn to -1
(Max unsigned int) if we hit overflow. It would be a simple patch
(later) to alter the existing logic to behave the same way, remove the
ST_OVERFLOW item, and display an overflow message when the count reaches
this value.
--Andrew Black
Log:
* makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp):
Add $(TEEOPTS) to compile/link line so that output is routed to log files.
* makefile.common (TEEOPTS): Always tee output to [EMAIL PROTECTED]
* target.h (target_opts): Add c_warn and l_warn fields.
(target_status): Split warn field into c_warn, l_warn, t_warn.
* output.cpp (check_test): Count warnings, store in status->t_warn.
(check_compat_test): Expand tail horizon, capture warnings into
status->t_warn.
* display.cpp (print_header_plain): Add WARN column, rename WALL
column to REAL.
(print_status_plain): Print the total number of warnings in the
WARN column.
* runall.cpp (count_warnings): Add static method to count the
number of warnings in a .log file
(check_target_ok): Always calculate object file name, call
count_warnings for object and executable.
* cmdopt.cpp (usage_text): Document new --warn switch
(parse_warn_opts): Define new helper function to parse --warn switch.
(eval_options): Used here in parsing and to initialize default
c_warn and l_warn values.
Index: etc/config/makefile.common
===================================================================
--- etc/config/makefile.common (revision 454401)
+++ etc/config/makefile.common (working copy)
@@ -143,8 +143,10 @@
# if LOGFILE is being created, tee command output into it
# IMPORTANT: $(TEEOPTS) must be last on the command line
-ifneq ($(LOGFILE),/dev/null)
- TEEOPTS = 2>&1 | tee -a $(LOGFILE)
+ifeq ($(LOGFILE),/dev/null)
+ TEEOPTS = 2>&1 | tee [EMAIL PROTECTED]
+else
+ TEEOPTS = 2>&1 | tee [EMAIL PROTECTED] | tee -a $(LOGFILE)
endif
# determine the name of the results file against which to compute regressions
Index: etc/config/makefile.rules
===================================================================
--- etc/config/makefile.rules (revision 454401)
+++ etc/config/makefile.rules (working copy)
@@ -60,16 +60,16 @@
ifneq ($(AS_EXT),".")
%.o: %$(AS_EXT)
- $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
+ $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
endif # ifneq ($(AS_EXT),".")
endif # ifneq ($(AS_EXT),)
%.o: %.cpp
- $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
+ $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
%: %.o
- $(LD) $< -o $@ $(LDFLAGS) $(LDLIBS)
+ $(LD) $< -o $@ $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
# disable compilation and linking in the same step
# %: %.cpp
@@ -78,7 +78,7 @@
# compile and link in one step to eliminate the space overhead of .o files
%: %.cpp
- $(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS)
+ $(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
endif # eq ($(NO_DOT_O),)
Index: util/cmdopt.cpp
===================================================================
--- util/cmdopt.cpp (revision 454401)
+++ util/cmdopt.cpp (working copy)
@@ -89,6 +89,7 @@
" --signal=sig Send itself the specified signal.\n"
" --ignore=sig Ignore the specified signal.\n"
" --ulimit=lim Set child process usage limits (see below).\n"
+ " --warn=alias Set compiler log warning pattern (see below).\n"
"\n"
" All short (single dash) options must be specified seperately.\n"
" If a short option takes a value, it may either be provided like\n"
@@ -121,6 +122,22 @@
" Note: Some operating systems lack support for some or all of the\n"
" ulimit modes. If a system is unable to limit a given property, a\n"
" warning message will be produced.\n"
+ "\n"
+ " --warn set the string used to parse compile and link logs. Rather\n"
+ " than specifying a search string, an alias code is provided,\n"
+ " coresponding to the output of a compiler and linker. Alias codes\n"
+ " are case sensitive.\n"
+ "\n"
+ " --warn modes:\n"
+ " acc HP aCC\n"
+ " cxx Compaq C++\n"
+ " eccp EDG eccp\n"
+ " gcc GNU gcc\n"
+ " icc Intel icc for Linux\n"
+ " mipspro SGI MIPSpro\n"
+ " sunpro Sun C++\n"
+ " vacpp IBM VisualAge C++\n"
+ " xlc IBM XLC++\n"
};
#if !defined (_WIN32) && !defined (_WIN64)
@@ -365,7 +382,52 @@
return 0;
}
+/**
+ Helper function to parse a warning value string
+ @param opts ulimit value string to pares
+ @see child_limits
+*/
+static bool
+parse_warn_opts (const char* arg, struct target_opts* defaults)
+{
+ static const struct {
+ const char* name;
+ const char* pat;
+ } warn_set [] = {
+ { "acc", "Warning " },
+/*
+ { "cds", "UNKNOWN"},
+ { "como", "UNKNOWN"},
+*/
+ { "cxx", "Warning:"},
+ { "eccp", "warning:"},
+ { "gcc", "warning:"},
+ { "icc", "warning #"},
+ { "mipspro", "CC: WARNING"},
+ { "sunpro", "Warning:"},
+ { "vacpp", ": (W) "},
+ { "xlc", ": (W) "}, /* xlc and vacpp are synonyms. */
+ { 0, 0 }
+ };
+
+ assert (0 != arg);
+ assert (0 != defaults);
+
+ for (size_t i = 0; warn_set [i].name; ++i) {
+ if (0 == strcmp (warn_set [i].name, arg)) {
+
+ /* Set both compiler and linker warning string. */
+ defaults->c_warn = warn_set [i].pat;
+ defaults->l_warn = warn_set [i].pat;
+
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
/**
Helper function to produce 'Bad argument' error message.
@@ -423,11 +485,41 @@
const char opt_signal[] = "--signal";
const char opt_sleep[] = "--sleep";
const char opt_ulimit[] = "--ulimit";
+ const char opt_warn[] = "--warn";
int i;
assert (0 != argv);
+ /* The chain of preprocesor logic below initializes the defaults->c_warn
+ and defaults->l_warn values.
+ */
+#ifdef __GNUG__
+ parse_warn_opts ("Gcc", defaults);
+#elif defined (__HP_aCC)
+ parse_warn_opts ("Acc", defaults);
+#elif defined (__IBMCPP__)
+ parse_warn_opts ("Xlc", defaults);
+#elif defined (__SUNPRO_CC)
+ parse_warn_opts ("Sunpro", defaults);
+#elif defined (SNI)
+ parse_warn_opts ("Cds", defaults);
+#elif defined (__APOGEE__) /* EDG variant that doesn't define __EDG__. */
+ parse_warn_opts ("Como", defaults);
+
+/* The following are EDG variants, that define __EDG__ */
+#elif defined (__DECCXX)
+ parse_warn_opts ("Cxx", defaults);
+#elif defined (_SGI_COMPILER_VERSION)
+ parse_warn_opts ("Mipspro", defaults);
+#elif defined (__INTEL_COMPILER)
+ parse_warn_opts ("Icc", defaults);
+
+/* So we need to check for __EDG__ after we check for them. */
+#elif defined (__EDG__)
+ parse_warn_opts ("Eccp", defaults);
+#endif
+
if (1 == argc || '-' != argv [1][0])
return 1;
@@ -563,6 +655,16 @@
}
}
}
+ else if ( sizeof opt_warn - 1 <= arglen
+ && !memcmp (opt_warn, argv [i], sizeof opt_warn - 1)) {
+ optname = opt_warn;
+ optarg = get_long_val (argv, &i, sizeof opt_warn - 1);
+ if (optarg && *optarg) {
+ if (!parse_warn_opts (optarg, defaults)) {
+ break;
+ }
+ }
+ }
/* fall through */
}
default:
Index: util/output.cpp
===================================================================
--- util/output.cpp (revision 454401)
+++ util/output.cpp (working copy)
@@ -109,7 +109,13 @@
return;
}
}
- /* else if (1 < r_lvl) warning*/
+ else if (1 < r_lvl) {
+ status->t_warn += r_active;
+ if (status->t_warn < r_active) {
+ status->t_warn = (unsigned int)-1;
+ return;
+ }
+ }
fmt_ok = 1;
}
}
@@ -138,7 +144,7 @@
assert (0 != data);
assert (0 != status);
- fseek (data, -64, SEEK_END); /* Seek near the end of the file */
+ fseek (data, -80, SEEK_END); /* Seek near the end of the file */
for (tok = fgetc (data); fsm < 4 && !feof (data); tok = fgetc (data)) {
switch (tok) {
@@ -160,13 +166,13 @@
fsm = 0;
}
}
-
- if (!feof (data)) { /* leading "## A" eaten above */
- read = fscanf (data, "ssertions = %u\n## FailedAssertions = %u",
- &status->assert, &status->failed);
+ if (!feof (data)) { /* leading "## W" eaten above */
+ read = fscanf (data, "arnings = %u\n## Assertions = %u\n"
+ "## FailedAssertions = %u",
+ &status->t_warn, &status->assert, &status->failed);
}
- if (2 != read) {
+ if (3 != read) {
status->status = ST_FORMAT;
}
}
Index: util/display.cpp
===================================================================
--- util/display.cpp (revision 454401)
+++ util/display.cpp (working copy)
@@ -40,8 +40,8 @@
*/
static void print_header_plain ()
{
- puts ("NAME STATUS ASSERTS FAILED PERCNT USER "
- "SYS WALL");
+ puts ("NAME STATUS WARN ASSERTS FAILED PERCNT "
+ "USER SYS REAL");
}
/**
@@ -76,6 +76,8 @@
else
printf (" 0");
+ printf (" %4u", status->c_warn + status->l_warn + status->t_warn);
+
/* Print assetions, if any registered */
if (status->assert) {
printf (" %7u %6u %5d%%", status->assert, status->failed,
Index: util/target.h
===================================================================
--- util/target.h (revision 454401)
+++ util/target.h (working copy)
@@ -98,6 +98,8 @@
char** argv; /**< Target argv array. */
unsigned timeout; /**< Allowed time for process execution. */
const char* data_dir; /**< Root dir for reference input/output files. */
+ const char* c_warn; /**< Warning flag string when compiling. */
+ const char* l_warn; /**< Warning flag string when linking. */
int compat; /**< Test suite compatability mode switch. */
rw_rlimit* core;
rw_rlimit* cpu;
@@ -141,7 +143,9 @@
const rw_timeval* user; /**< Elapsed user time spent in execution. */
const rw_timeval* sys; /**< Elapsed system time spent in execution. */
const rw_timeval* wall; /**< Wall clock time spent in execution. */
- unsigned warn; /**< Number of (test) warnings. */
+ unsigned c_warn; /**< Number of compile warnings. */
+ unsigned l_warn; /**< Number of link warnings. */
+ unsigned t_warn; /**< Number of (test) warnings. */
unsigned assert; /**< Number of (test) assertions. */
unsigned failed; /**< Number of failed (test) assertions. */
};
Index: util/runall.cpp
===================================================================
--- util/runall.cpp (revision 454401)
+++ util/runall.cpp (working copy)
@@ -28,6 +28,7 @@
#include <errno.h> /* for errno */
#include <stdlib.h> /* for exit(), free() */
#include <string.h> /* for memcpy(), ... */
+#include <stdio.h> /* for FILE, fopen(), ... */
#include <ctype.h> /* for isspace */
#include <sys/types.h>
@@ -193,11 +194,82 @@
}
/**
+ Arbitrary constant controling static read buffer size.
+
+ @see count_warnings
+*/
+#define READ_BUF_LEN 64
+
+/**
+ Compiler/linker output parser.
+
+ This method tries to open the compiler/linker log file, based on the name
+ of a the target file that was generated by the compiler/linker.
+ Upon opening the file, it tries to count the number of warnings present.
+
+ @param target name of target to parse the log for
+ @param counter pointer to the counter to count warnings in.
+ @param match string to match when detecting a warning.
+*/
+static
+void count_warnings (const char* const target, unsigned* counter,
+ const char* const match)
+{
+ size_t path_len;
+ char* tmp_name;
+ FILE* data;
+
+ assert (0 != target);
+ assert (0 != counter);
+
+ if (0 == match)
+ return; /* Don't have a match string, so don't tally warnings */
+
+ path_len = strlen (target);
+ tmp_name = (char*)RW_MALLOC (path_len + 5);
+ memcpy (tmp_name, target, path_len + 1);
+ strcat (tmp_name,".log");
+
+ data = fopen (tmp_name, "r");
+ if (data) {
+ size_t pos = 0, limit = strlen (match);
+ while (!feof (data)) {
+ char buf [READ_BUF_LEN];
+ size_t nread; /* bytes read */
+
+ /* First, read a block from the file into the buffer */
+ nread = fread (buf, 1, sizeof buf, data);
+ if (ferror (data)) {
+ warn ("Error reading %s: %s\n", tmp_name, strerror (errno));
+ break;
+ }
+
+ /* Then, look for the search string within it. */
+ for (size_t i = 0; i < nread; ++i) {
+ if (buf [i] != match [pos])
+ pos = 0;
+ else if (++pos == limit) {
+ pos = 0;
+ ++*counter;
+ }
+ }
+ }
+
+ fclose (data);
+ }
+ else if (ENOENT != errno)
+ warn ("Error opening %s: %s\n", tmp_name, strerror (errno));
+
+ free (tmp_name);
+}
+
+/**
Preflight check to ensure that target is something that should be run.
This method checks to see if target exists and is theoretically executable.
If a problem is detected, the condition is recorded in the status structure
- and 0 is returned.
+ and 0 is returned. This method also attempts to determine the number of
+ compile and link warnings that occurred, using count_warnings.
A special case is the situation where the target name ends in .o,
indicating that the target only needed to compile, and doesn't need to
@@ -212,31 +284,63 @@
{
struct stat file_info;
int exists = 1;
+ size_t path_len;
+ char* tmp_name;
assert (0 != target);
assert (0 != status);
+ path_len = strlen (target);
+
+#ifndef _WIN32
+ /* Otherwise, check for the .o file on non-windows systems */
+ tmp_name = (char*)RW_MALLOC (path_len + 3);
+ memcpy (tmp_name, target, path_len + 1);
+ strcat (tmp_name,".o");
+#else
+ /* Or the target\target.obj file on windows systems*/
+ {
+ const char* const target_name = get_target ();
+ size_t target_len = strlen (target_name);
+ size_t tmp_len = path_len + target_len - 2;
+ /* - 2 comes from removing 4 characters (extra .exe) and adding 2
+ characters (\ directory seperator and trailing null) */
+ tmp_name = (char*)RW_MALLOC (tmp_len);
+ memcpy (tmp_name, target, path_len - 4);
+ tmp_name [path_len - 4] = default_path_sep;
+ memcpy (tmp_name + path_len - 3, target_name, target_len);
+ tmp_name [tmp_len - 4] = 'o';
+ tmp_name [tmp_len - 3] = 'b';
+ tmp_name [tmp_len - 2] = 'j';
+ tmp_name [tmp_len - 2] = '\0';
+ }
+#endif /* _WIN32 */
+
+ count_warnings (target, &status->l_warn, "warning:");
+ count_warnings (tmp_name, &status->c_warn, "warning:");
+
if (0 > stat (target, &file_info)) {
if (ENOENT != errno) {
warn ("Error stating %s: %s\n", target, strerror (errno));
status->status = ST_SYSTEM_ERROR;
+ free (tmp_name);
return 0;
}
file_info.st_mode = 0; /* force mode on non-existant file to 0 */
exists = 0; /* note that it doesn't exist */
}
+
if (0 == (file_info.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))) {
/* This is roughly equivlent to the -x bash operator.
It checks if the file can be run, /not/ if we can run it
*/
- const size_t path_len = strlen (target);
- char* tmp_name;
#if 0 /* Disable .o target check as unused */
/* If target is a .o file, check if it exists */
if ('.' == target [path_len-1] && 'o' == target [path_len]) {
if (!exists)
status->status = ST_COMPILE;
+ free (tmp_name);
return 0;
}
#endif
@@ -244,34 +348,10 @@
/* If the target exists, it doesn't have valid permissions */
if (exists) {
status->status = ST_EXECUTE_FLAG;
+ free (tmp_name);
return 0;
}
-#if !defined (_WIN32) && !defined (_WIN64)
- /* Otherwise, check for the .o file on non-windows systems */
- tmp_name = (char*)RW_MALLOC (path_len + 3);
- memcpy (tmp_name, target, path_len + 1);
- strcat (tmp_name,".o");
-#else
- /* Or the target\target.obj file on windows systems*/
- {
- const char* const target_name = get_target ();
- size_t target_len = strlen (target_name);
- size_t tmp_len = path_len + target_len - 2;
- /* - 2 comes from removing 4 characters (extra .exe) and
- adding 2 characters (\ directory seperator and trailing
- null) */
- tmp_name = (char*)RW_MALLOC (tmp_len);
- memcpy (tmp_name, target, path_len - 4);
- tmp_name [path_len - 4] = default_path_sep;
- memcpy (tmp_name + path_len - 3, target_name, target_len);
- tmp_name [tmp_len - 4] = 'o';
- tmp_name [tmp_len - 3] = 'b';
- tmp_name [tmp_len - 2] = 'j';
- tmp_name [tmp_len - 2] = '\0';
- }
-#endif /* _WIN{32,64} */
-
if (0 > stat (tmp_name, &file_info)) {
if (ENOENT != errno) {
warn ("Error stating %s: %s\n", tmp_name, strerror (errno));
@@ -287,6 +367,7 @@
free (tmp_name);
return 0;
}
+ free (tmp_name);
return 1;
}
