This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [patch/rfc] complaint() -> complaints() rewrite #2
- From: Andrew Cagney <ac131313 at ges dot redhat dot com>
- To: Andrew Cagney <ac131313 at ges dot redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Wed, 18 Sep 2002 20:41:28 -0400
- Subject: Re: [patch/rfc] complaint() -> complaints() rewrite #2
- References: <3D5B05A9.3060102@ges.redhat.com>
Hello,
This follows up an earler e-mail where I wrote:
Should this be called symtab_complaint(), or perhaphs, should complaint() take an extra parameter (the complaint class) so that other code can use this mechanism?
I've implemented this. In the process I've tried to make the code a little bit more i18n friendly (it was assuming that things read left to right).
Anyway, this is my original description:
The files complaints.[hc] implement a mechanism for supressing warning messages that occure more than a small number of times. GDB's debug readers use this mechanism to supress all but the first few warnings generated when reading an object file.
The current implementation stores the warning message string in a structure vis:
struct complaint argument_complaint = { "Argument '%d'", };
DOUBLEST argument;
complain (&argument_complaint, "argument");
complain (&argument_complaint, argument);
complain (&argument_complaint, &argument);
The problem I see with this is that there is nothing (other than the human eye) checking for consistency between the format string and the call.
I'd like to propose a new complaints interface:
void complaint (const char *fmt, ...);
(with a format printf attribute) so that the compiler (GCC with -Wformat) can check, at build time, the consistency of the format string and its parameter list. To issue the same complaint from multiple points in the code, a wrapper function can be used:
argument_complaint (int argument)
{
complaint ("Argument '%d'", argument);
}
The process of updating the above should flush out a few bugs :-)
This change would help eliminate problems such as:
http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=211
--
I don't use the most efficient of algorithms when detecting duplicate complaints. But then, I don't know how often complaints are occuring and my objective is to fix the format miss-match.
The complaint() interface should probably be documented along with error() and warning().
--
thoughts?
Andrew
I've checked the attached in. It's slightly different to the original
in that:
- it includes internal_complaint() to match intenal_error() and
internal_warning()
- complain() simply re-calls complaint() with the message
Thanks to Kevin for giving it the once over.
Attached is the final patch,
Andrew
2002-09-18 Andrew Cagney <ac131313@redhat.com>
* complaints.h: Update copyright.
(struct complaints): Declare.
(struct complaint): Make `message' constant.
(internal_complaint): Declare.
(complaint): Declare.
(complaint_root): Delete declaration.
(symfile_complaints): Delete declaration.
(struct complaints): Add opaque declaration.
(clear_complaints): Add a complaints parameter.
* complaints.c: Update copyright.
(enum complaint_series): Define.
(complaint_root): Delete.
(struct complaints): Define.
(complaint_sentinel, symfile_complaint_book): New variables.
(symfile_explanations, symfile_complaints): New variables.
New variables.
(get_complaints): New function.
(vcomplaint): New function.
(complaint): New function.
(internal_complaint): New function.
(complain): Call vcomplain with symfile_complaint.
(clear_complaints): Rewrite.
(_initialize_complaints): Use add_setshow_command.
* Makefile.in (complaints.o): Update dependencies.
* symfile.c (syms_from_objfile): Add symfile_complaints parameter
to call to clear_complaints.
(new_symfile_objfile, reread_symbols): Ditto.
(oldsyms_complaint): Delete.
(empty_symtab_complaint, unknown_option_complaint): Delete.
(free_named_symtabs): Use complaint instead of complain.
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.259
diff -u -r1.259 Makefile.in
--- Makefile.in 17 Sep 2002 04:44:21 -0000 1.259
+++ Makefile.in 19 Sep 2002 00:38:57 -0000
@@ -1572,7 +1572,8 @@
$(coff_internal_h) $(libcoff_h) $(symfile_h) $(objfiles_h) \
$(buildsym_h) $(gdb_stabs_h) $(stabsread_h) $(complaints_h) \
$(target_h) $(gdb_assert_h)
-complaints.o: complaints.c $(defs_h) $(complaints_h) $(gdbcmd_h)
+complaints.o: complaints.c $(defs_h) $(complaints_h) $(gdb_assert_h) \
+ $(command_h) $(gdbcmd_h)
completer.o: completer.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(expression_h) \
$(filenames_h) $(cli_decode_h) $(gdbcmd_h) $(completer_h)
copying.o: copying.c $(defs_h) $(command_h) $(gdbcmd_h)
Index: complaints.c
===================================================================
RCS file: /cvs/src/src/gdb/complaints.c,v
retrieving revision 1.6
diff -u -r1.6 complaints.c
--- complaints.c 6 Nov 2001 23:38:14 -0000 1.6
+++ complaints.c 19 Sep 2002 00:38:57 -0000
@@ -1,6 +1,7 @@
/* Support for complaint handling during symbol reading in GDB.
- Copyright 1990, 1991, 1992, 1993, 1995, 1998, 1999, 2000
- Free Software Foundation, Inc.
+
+ Copyright 1990, 1991, 1992, 1993, 1995, 1998, 1999, 2000, 2002 Free
+ Software Foundation, Inc.
This file is part of GDB.
@@ -21,148 +22,273 @@
#include "defs.h"
#include "complaints.h"
+#include "gdb_assert.h"
+#include "command.h"
#include "gdbcmd.h"
extern void _initialize_complaints (void);
+/* Should each complaint message be self explanatory, or should we assume that
+ a series of complaints is being produced? */
+
+/* case 1: First message of a series that must
+ start off with explanation. case 2: Subsequent message of a series
+ that needs no explanation (the user already knows we have a problem
+ so we can just state our piece). */
+enum complaint_series {
+ /* Isolated self explanatory message. */
+ ISOLATED_MESSAGE,
+ /* First message of a series, includes an explanation. */
+ FIRST_MESSAGE,
+ /* First message of a series, but does not need to include any sort
+ of explanation. */
+ SHORT_FIRST_MESSAGE,
+ /* Subsequent message of a series that needs no explanation (the
+ user already knows we have a problem so we can just state our
+ piece). */
+ SUBSEQUENT_MESSAGE
+};
+
/* Structure to manage complaints about symbol file contents. */
-struct complaint complaint_root[1] =
+struct complain
{
- {
- (char *) NULL, /* Complaint message */
- 0, /* Complaint counter */
- complaint_root /* Next complaint. */
- }
+ const char *file;
+ int line;
+ const char *fmt;
+ int counter;
+ struct complain *next;
};
-/* How many complaints about a particular thing should be printed before
- we stop whining about it? Default is no whining at all, since so many
- systems have ill-constructed symbol files. */
+struct complaints
+{
+ struct complain *root;
-static unsigned int stop_whining = 0;
+ /* Should each complaint be self explanatory, or should we assume
+ that a series of complaints is being produced? case 0: Isolated
+ self explanatory message. case 1: First message of a series that
+ must start off with explanation. case 2: Subsequent message of a
+ series that needs no explanation (the user already knows we have
+ a problem so we can just state our piece). */
+ int series;
+
+ /* The explanatory messages that should accompany the complaint.
+ NOTE: cagney/2002-08-14: In a desperate attempt at being vaguely
+ i18n friendly, this is an array of two messages. When present,
+ EXPLANATION[SERIES] is used to wrap the message. */
+ const char **explanation;
+};
-/* Should each complaint be self explanatory, or should we assume that
- a series of complaints is being produced?
- case 0: self explanatory message.
- case 1: First message of a series that must start off with explanation.
- case 2: Subsequent message, when user already knows we are reading
- symbols and we can just state our piece. */
+static struct complain complaint_sentinel;
-static int complaint_series = 0;
+/* The symbol table complaint table. */
-
+static const char *symfile_explanations[] = {
+ "During symbol reading, %s.\n",
+ "During symbol reading...%s...",
+ "%s...",
+ "%s...",
+ NULL
+};
-/* Functions to handle complaints during symbol reading. */
+static struct complaints symfile_complaint_book = {
+ &complaint_sentinel,
+ 0,
+ symfile_explanations
+};
+struct complaints *symfile_complaints = &symfile_complaint_book;
-/* Print a complaint about the input symbols, and link the complaint block
- into a chain for later handling. */
+/* Wrapper function to, on-demand, fill in a complaints object. */
-void
-complain (struct complaint *complaint,...)
+static struct complaints *
+get_complaints (struct complaints **c)
{
- va_list args;
- va_start (args, complaint);
+ if ((*c) != NULL)
+ return (*c);
+ (*c) = XMALLOC (struct complaints);
+ (*c)->root = &complaint_sentinel;
+ (*c)->series = ISOLATED_MESSAGE;
+ (*c)->explanation = NULL;
+ return (*c);
+}
- complaint->counter++;
- if (complaint->next == NULL)
+static struct complain *
+find_complaint (struct complaints *complaints, const char *file,
+ int line, const char *fmt)
+{
+ struct complain *complaint;
+
+ /* Find the complaint in the table. A more efficient search
+ algorithm (based on hash table or something) could be used. But
+ that can wait until someone shows evidence that this lookup is
+ a real bottle neck. */
+ for (complaint = complaints->root;
+ complaint != NULL;
+ complaint = complaint->next)
{
- complaint->next = complaint_root->next;
- complaint_root->next = complaint;
+ if (complaint->fmt == fmt
+ && complaint->file == file
+ && complaint->line == line)
+ return complaint;
}
+
+ /* Oops not seen before, fill in a new complaint. */
+ complaint = XMALLOC (struct complain);
+ complaint->fmt = fmt;
+ complaint->file = file;
+ complaint->line = line;
+ complaint->counter = 0;
+ complaint->next = NULL;
+
+ /* File it, return it. */
+ complaint->next = complaints->root;
+ complaints->root = complaint;
+ return complaint;
+}
+
+
+/* How many complaints about a particular thing should be printed
+ before we stop whining about it? Default is no whining at all,
+ since so many systems have ill-constructed symbol files. */
+
+static unsigned int stop_whining = 0;
+
+/* Print a complaint, and link the complaint block into a chain for
+ later handling. */
+
+static void
+vcomplaint (struct complaints **c, const char *file, int line, const char *fmt,
+ va_list args)
+{
+ struct complaints *complaints = get_complaints (c);
+ struct complain *complaint = find_complaint (complaints, file, line, fmt);
+ enum complaint_series series;
+ gdb_assert (complaints != NULL);
+
+ complaint->counter++;
if (complaint->counter > stop_whining)
+ return;
+
+ if (info_verbose)
+ series = SUBSEQUENT_MESSAGE;
+ else
+ series = complaints->series;
+
+ if (complaint->file != NULL)
+ internal_vwarning (complaint->file, complaint->line, complaint->fmt, args);
+ else if (warning_hook)
+ (*warning_hook) (complaint->fmt, args);
+ else
{
- return;
+ if (complaints->explanation == NULL)
+ vwarning (complaint->fmt, args);
+ else
+ {
+ char *msg;
+ struct cleanup *cleanups;
+ xvasprintf (&msg, complaint->fmt, args);
+ cleanups = make_cleanup (xfree, msg);
+ wrap_here ("");
+ if (series != SUBSEQUENT_MESSAGE)
+ begin_line ();
+ fprintf_filtered (gdb_stderr,
+ complaints->explanation[series],
+ msg);
+ wrap_here ("");
+ do_cleanups (cleanups);
+ }
}
- wrap_here ("");
- switch (complaint_series + (info_verbose << 1))
+ switch (series)
{
-
- /* Isolated messages, must be self-explanatory. */
- case 0:
- if (warning_hook)
- (*warning_hook) (complaint->message, args);
- else
- {
- begin_line ();
- fputs_filtered ("During symbol reading, ", gdb_stderr);
- wrap_here ("");
- vfprintf_filtered (gdb_stderr, complaint->message, args);
- fputs_filtered (".\n", gdb_stderr);
- }
+ case ISOLATED_MESSAGE:
break;
-
- /* First of a series, without `set verbose'. */
- case 1:
- if (warning_hook)
- (*warning_hook) (complaint->message, args);
- else
- {
- begin_line ();
- fputs_filtered ("During symbol reading...", gdb_stderr);
- vfprintf_filtered (gdb_stderr, complaint->message, args);
- fputs_filtered ("...", gdb_stderr);
- wrap_here ("");
- complaint_series++;
- }
+ case FIRST_MESSAGE:
+ complaints->series = SUBSEQUENT_MESSAGE;
+ break;
+ case SUBSEQUENT_MESSAGE:
+ case SHORT_FIRST_MESSAGE:
+ complaints->series = SUBSEQUENT_MESSAGE;
break;
-
- /* Subsequent messages of a series, or messages under `set verbose'.
- (We'll already have produced a "Reading in symbols for XXX..."
- message and will clean up at the end with a newline.) */
- default:
- if (warning_hook)
- (*warning_hook) (complaint->message, args);
- else
- {
- vfprintf_filtered (gdb_stderr, complaint->message, args);
- fputs_filtered ("...", gdb_stderr);
- wrap_here ("");
- }
}
- /* If GDB dumps core, we'd like to see the complaints first. Presumably
- GDB will not be sending so many complaints that this becomes a
- performance hog. */
+
+ /* If GDB dumps core, we'd like to see the complaints first.
+ Presumably GDB will not be sending so many complaints that this
+ becomes a performance hog. */
+
gdb_flush (gdb_stderr);
+}
+
+void
+complaint (struct complaints **complaints, const char *fmt, ...)
+{
+ va_list args;
+ va_start (args, fmt);
+ vcomplaint (complaints, NULL/*file*/, 0/*line*/, fmt, args);
+ va_end (args);
+}
+
+void
+internal_complaint (struct complaints **complaints, const char *file,
+ int line, const char *fmt, ...)
+{
+ va_list args;
+ va_start (args, fmt);
+ vcomplaint (complaints, file, line, fmt, args);
+ va_end (args);
+}
+
+void
+complain (struct complaint *complaint, ...)
+{
+ va_list args;
+ va_start (args, complaint);
+ vcomplaint (&symfile_complaints, NULL/*file*/, 0/*line*/,
+ complaint->message, args);
va_end (args);
}
-/* Clear out all complaint counters that have ever been incremented.
- If sym_reading is 1, be less verbose about successive complaints,
- since the messages are appearing all together during a command that
- reads symbols (rather than scattered around as psymtabs get fleshed
- out into symtabs at random times). If noisy is 1, we are in a
- noisy symbol reading command, and our caller will print enough
- context for the user to figure it out. */
+/* Clear out / initialize all complaint counters that have ever been
+ incremented. If LESS_VERBOSE is 1, be less verbose about
+ successive complaints, since the messages are appearing all
+ together during a command that is reporting a contiguous block of
+ complaints (rather than being interleaved with other messages). If
+ noisy is 1, we are in a noisy command, and our caller will print
+ enough context for the user to figure it out. */
void
-clear_complaints (int sym_reading, int noisy)
+clear_complaints (struct complaints **c, int less_verbose, int noisy)
{
- struct complaint *p;
+ struct complaints *complaints = get_complaints (c);
+ struct complain *p;
- for (p = complaint_root->next; p != complaint_root; p = p->next)
+ for (p = complaints->root; p != NULL; p = p->next)
{
p->counter = 0;
}
- if (!sym_reading && !noisy && complaint_series > 1 && !warning_hook)
+ if (complaints->series > 1 && !warning_hook)
{
/* Terminate previous series, since caller won't. */
puts_filtered ("\n");
}
- complaint_series = sym_reading ? 1 + noisy : 0;
+ if (!less_verbose)
+ complaints->series = ISOLATED_MESSAGE;
+ else if (!noisy)
+ complaints->series = FIRST_MESSAGE;
+ else
+ complaints->series = SHORT_FIRST_MESSAGE;
}
void
_initialize_complaints (void)
{
- add_show_from_set
- (add_set_cmd ("complaints", class_support, var_zinteger,
- (char *) &stop_whining,
- "Set max number of complaints about incorrect symbols.",
- &setlist),
- &showlist);
+ add_setshow_cmd ("complaints", class_support, var_zinteger,
+ &stop_whining,
+ "Set max number of complaints about incorrect symbols.",
+ "Show max number of complaints about incorrect symbols.",
+ NULL, NULL,
+ &setlist, &showlist);
}
Index: complaints.h
===================================================================
RCS file: /cvs/src/src/gdb/complaints.h,v
retrieving revision 1.3
diff -u -r1.3 complaints.h
--- complaints.h 6 Mar 2001 08:21:06 -0000 1.3
+++ complaints.h 19 Sep 2002 00:38:57 -0000
@@ -1,6 +1,7 @@
/* Definitions for complaint handling during symbol reading in GDB.
- Copyright 1990, 1991, 1992, 1995, 1998, 2000
- Free Software Foundation, Inc.
+
+ Copyright 1990, 1991, 1992, 1995, 1998, 2000, 2002 Free Software
+ Foundation, Inc.
This file is part of GDB.
@@ -23,8 +24,37 @@
#if !defined (COMPLAINTS_H)
#define COMPLAINTS_H
+/* Opaque object used to track the number of complaints of a
+ particular category. */
+struct complaints;
+
+/* Predefined categories. */
+extern struct complaints *symfile_complaints;
+
+/* Register a complaint. */
+extern void complaint (struct complaints **complaints, const char *fmt,
+ ...) ATTR_FORMAT (printf, 2, 3);
+extern void internal_complaint (struct complaints **complaints,
+ const char *file, int line, const char *fmt,
+ ...) ATTR_FORMAT (printf, 4, 5);
+
+/* Clear out / initialize all complaint counters that have ever been
+ incremented. If LESS_VERBOSE is 1, be less verbose about
+ successive complaints, since the messages are appearing all
+ together during a command that is reporting a contiguous block of
+ complaints (rather than being interleaved with other messages). If
+ noisy is 1, we are in a noisy command, and our caller will print
+ enough context for the user to figure it out. */
+
+extern void clear_complaints (struct complaints **complaints,
+ int less_verbose, int noisy);
+
+
+/* Legacy interfaces to keep the old code working (until it is all
+ converted to the above). While the structure below contains a
+ number of fields, all but .message are ignored.
-/* Support for complaining about things in the symbol file that aren't
+ Support for complaining about things in the symbol file that aren't
catastrophic.
Each such thing gets a counter. The first time we have the problem,
@@ -32,22 +62,12 @@
if verbose, we report how many of each problem we had. */
struct complaint
- {
- char *message;
- unsigned counter;
- struct complaint *next;
- };
-
-/* Root of the chain of complaints that have at some point been issued.
- This is used to reset the counters, and/or report the total counts. */
-
-extern struct complaint complaint_root[1];
-
-/* Functions that handle complaints. (in complaints.c) */
+{
+ const char *message;
+ unsigned counter_ignored;
+ struct complaint *next_ignored;
+};
extern void complain (struct complaint *, ...);
-
-extern void clear_complaints (int, int);
-
#endif /* !defined (COMPLAINTS_H) */
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.65
diff -u -r1.65 symfile.c
--- symfile.c 1 Aug 2002 17:18:32 -0000 1.65
+++ symfile.c 19 Sep 2002 00:38:57 -0000
@@ -81,21 +81,6 @@
/* Global variables owned by this file */
int readnow_symbol_files; /* Read full symbols immediately */
-struct complaint oldsyms_complaint =
-{
- "Replacing old symbols for `%s'", 0, 0
-};
-
-struct complaint empty_symtab_complaint =
-{
- "Empty symbol table found for `%s'", 0, 0
-};
-
-struct complaint unknown_option_complaint =
-{
- "Unknown option `%s' ignored", 0, 0
-};
-
/* External variables and functions referenced. */
extern void report_transfer_performance (unsigned long, time_t, time_t);
@@ -706,7 +691,7 @@
initial symbol reading for this file. */
(*objfile->sf->sym_init) (objfile);
- clear_complaints (1, verbo);
+ clear_complaints (&symfile_complaints, 1, verbo);
(*objfile->sf->sym_offsets) (objfile, addrs);
@@ -818,7 +803,7 @@
}
/* We're done reading the symbol file; finish off complaints. */
- clear_complaints (0, verbo);
+ clear_complaints (&symfile_complaints, 0, verbo);
}
/* Process a symbol file, as either the main file or as a dynamically
@@ -1801,7 +1786,7 @@
}
(*objfile->sf->sym_init) (objfile);
- clear_complaints (1, 1);
+ clear_complaints (&symfile_complaints, 1, 1);
/* The "mainline" parameter is a hideous hack; I think leaving it
zero is OK since dbxread.c also does what it needs to do if
objfile->global_psymbols.size is 0. */
@@ -1815,7 +1800,7 @@
objfile->flags |= OBJF_SYMS;
/* We're done reading the symbol file; finish off complaints. */
- clear_complaints (0, 1);
+ clear_complaints (&symfile_complaints, 0, 1);
/* Getting new symbols may change our opinion about what is
frameless. */
@@ -2305,15 +2290,16 @@
|| BLOCK_NSYMS (BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK))
|| BLOCK_NSYMS (BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK)))
{
- complain (&oldsyms_complaint, name);
-
+ complaint (&symfile_complaints, "Replacing old symbols for `%s'",
+ name);
clear_symtab_users_queued++;
make_cleanup (clear_symtab_users_once, 0);
blewit = 1;
}
else
{
- complain (&empty_symtab_complaint, name);
+ complaint (&symfile_complaints, "Empty symbol table found for `%s'",
+ name);
}
free_symtab (s);