|
From: <sv...@va...> - 2008-06-30 08:42:51
|
Author: bart
Date: 2008-06-30 09:42:58 +0100 (Mon, 30 Jun 2008)
New Revision: 8311
Log:
Added function VG_(ToXML)().
Modified:
branches/FORMATCHECK/coregrind/m_debuglog.c
Modified: branches/FORMATCHECK/coregrind/m_debuglog.c
===================================================================
--- branches/FORMATCHECK/coregrind/m_debuglog.c 2008-06-30 08:41:59 UTC (rev 8310)
+++ branches/FORMATCHECK/coregrind/m_debuglog.c 2008-06-30 08:42:58 UTC (rev 8311)
@@ -49,6 +49,8 @@
#include "pub_core_basics.h" /* basic types */
#include "pub_core_vkiscnums.h" /* for syscall numbers */
#include "pub_core_debuglog.h" /* our own iface */
+#include "pub_core_libcassert.h" /* vg_assert() */
+#include "pub_tool_libcprint.h" /* VG_(ToXML)() */
#include "valgrind.h" /* for RUNNING_ON_VALGRIND */
/*------------------------------------------------------------*/
@@ -501,7 +503,7 @@
static
UInt myvprintf_str_XML_simplistic ( void(*send)(HChar,void*),
void* send_arg2,
- HChar* str )
+ const HChar* str )
{
UInt ret = 0;
Int i;
@@ -879,8 +881,29 @@
va_end(vargs);
}
+static HChar xml_result_buf[256];
+static int xml_result_buf_pos;
+static void append_to_xml_result_buf(const HChar ch, void* arg2)
+{
+ if (xml_result_buf_pos
+ <= sizeof(xml_result_buf) / sizeof(xml_result_buf[0]) - 2)
+ {
+ xml_result_buf[xml_result_buf_pos++] = ch;
+ }
+}
+/* Convert a string such that it can be inserted into an XML output stream. */
+extern HChar* VG_(ToXML)(const HChar* str)
+{
+ if (str == NULL)
+ str = "(null)";
+ xml_result_buf_pos = 0;
+ myvprintf_str_XML_simplistic(append_to_xml_result_buf, NULL, str);
+ xml_result_buf[xml_result_buf_pos] = 0;
+ return xml_result_buf;
+}
+
/*--------------------------------------------------------------------*/
/*--- end m_debuglog.c ---*/
/*--------------------------------------------------------------------*/
|
|
From: Dirk M. <dm...@gm...> - 2008-06-30 22:24:01
|
On Monday 30 June 2008, sv...@va... wrote: > Author: bart > Date: 2008-06-30 09:42:58 +0100 (Mon, 30 Jun 2008) > New Revision: 8311 > > Log: > Added function VG_(ToXML)(). Hi, thanks for reviving the branch. this was actually an open question to Julian: is it guaranteed that the printf code is never used from multiple threads? otherwise the static buffer used here in the ToXML() function hurts. Greetings, Dirk |
|
From: Julian S. <js...@ac...> - 2008-07-01 04:16:36
|
> thanks for reviving the branch. this was actually an open question to > Julian: is it guaranteed that the printf code is never used from multiple > threads? otherwise the static buffer used here in the ToXML() function > hurts. Er, no, I don't think it's thread safe, or at least it would be difficult to construct an argument that it would. It would be much better to have the caller allocate the buffer. Another thing is that this appears to break the module structure: +#include "pub_core_libcassert.h" /* vg_assert() */ +#include "pub_tool_libcprint.h" /* VG_(ToXML)() */ If this function's prototype is in pub_tool_libcprint.h then its implementation should be in m_libcprint, not in m_debuglog. Also, adding extra includes to m_debuglog.c is not allowed, as per big comment at the top of that file. As a result of uncontrolled #include-ing, earlier in the life of the project there was a circular dependency between m_aspacemgr and m_mallocfree, which caused a lot of trouble and was only removed at great effort. What is the reason for needing VG_(ToXML) anyway? I thought the XML output stuff worked OK as it is. J |
|
From: Bart V. A. <bar...@gm...> - 2008-07-01 07:49:55
|
On Tue, Jul 1, 2008 at 6:09 AM, Julian Seward <js...@ac...> wrote: > It would be much better to have the caller allocate the buffer. The function VG_(ToXML)() now expects the caller to allocate the buffer. I'm not sure whether the sizes I reserved for these buffers are large enough. > If this function's prototype is in pub_tool_libcprint.h then its > implementation should be in m_libcprint, not in m_debuglog. Fixed. > Also, adding extra includes to m_debuglog.c is not allowed, as per > big comment at the top of that file. Fixed. > What is the reason for needing VG_(ToXML) anyway? I thought the > XML output stuff worked OK as it is. A quote from gcc's -Wformat documentation (from the gcc 4.3.1 manual): "The formats are checked against the format features supported by GNU libc version 2.2. These include all ISO C90 and C99 features, as well as features from the Single Unix Specification and some BSD and GNU extensions." The %t format specifier, used for XML-escaping, is a Valgrind-extension and is not understood by gcc's format checker. That is why I replaced "...%t...", ..., str, ... by "...%s...", ..., ToXML(str), ... Bart. |
|
From: Dirk M. <dm...@gm...> - 2008-07-01 08:03:04
|
On Tuesday 01 July 2008, Julian Seward wrote: > Er, no, I don't think it's thread safe, or at least it would be difficult > to construct an argument that it would. It would be much better to have > the caller allocate the buffer. Uglifies the code.. a bit.. > What is the reason for needing VG_(ToXML) anyway? I thought the > XML output stuff worked OK as it is. VG_(printf) uses "%t" as a "xml-escaped %s" attribute. This does not if gcc format checking is enabled, as %t is a nonstandard extension, which it doesn't understand. I've asked you some time ago wether it would be okay to either map it to "%s" and use a ToXml function for the argument, or to use a completely different (non printf-like) function for xml output. I guess the latter makes more sense, as line formatting (printf) doesn't really make sense with xml output, the stream output can be abstracted into something else. Greetings, Dirk |
|
From: Bart V. A. <bar...@gm...> - 2008-07-02 12:08:27
|
On Tue, Jul 1, 2008 at 10:02 AM, Dirk Mueller <dm...@gm...> wrote:
> VG_(printf) uses "%t" as a "xml-escaped %s" attribute. This does not if gcc
> format checking is enabled, as %t is a nonstandard extension, which it doesn't
> understand. I've asked you some time ago wether it would be okay to either map
> it to "%s" and use a ToXml function for the argument, or to use a completely
> different (non printf-like) function for xml output. I guess the latter makes
> more sense, as line formatting (printf) doesn't really make sense with xml
> output, the stream output can be abstracted into something else.
How about the code below ? The patch below defines two variants of the
same function, namely VG_(xml_message)() and VG_(message)(). Format
string checking is disabled for the former and enabled for the latter.
This way the %t format specifier does not trigger a warning when
passed to VG_(xml_message)().
Bart.
Index: include/pub_tool_libcprint.h
===================================================================
--- include/pub_tool_libcprint.h (revision 8323)
+++ include/pub_tool_libcprint.h (working copy)
@@ -85,12 +85,11 @@
VgMsgKind;
/* Send a single-part message. Appends a newline. */
+extern UInt VG_(xml_message)( VgMsgKind kind, const HChar* format, ... );
extern UInt VG_(message) ( VgMsgKind kind, const HChar* format,
... ) PRINTF_CHECK(2, 3);
extern UInt VG_(vmessage) ( VgMsgKind kind, const HChar* format,
va_list vargs ) PRINTF_CHECK(2, 0);
-/* Convert a string such that it can be inserted into an XML output stream. */
-extern HChar* VG_(ToXML)(HChar* buf, Int size, const HChar* str);
#endif // __PUB_TOOL_LIBCPRINT_H
Index: coregrind/m_libcprint.c
===================================================================
--- coregrind/m_libcprint.c (revision 8323)
+++ coregrind/m_libcprint.c (working copy)
@@ -285,47 +285,6 @@
/* ---------------------------------------------------------------------
- ToXML()
- ------------------------------------------------------------------ */
-
-struct xml_buf {
- HChar* cur;
- HChar* end;
-};
-
-static void append_to_xml_buf(const HChar ch, void* arg2)
-{
- struct xml_buf* const xml_buf = arg2;
-
- if (xml_buf->cur < xml_buf->end)
- {
- *xml_buf->cur++ = ch;
- }
- else
- {
- /* Make sure that it gets noticed if a buffer is too small. */
- vg_assert(False);
- }
-}
-
-/* Convert a string such that it can be inserted into an XML output stream. */
-
-extern HChar* VG_(ToXML)(HChar* const buf, const Int size,
- const HChar* str)
-{
- static struct xml_buf xml_buf;
-
- if (str == NULL)
- str = "(null)";
- xml_buf.cur = buf;
- xml_buf.end = buf + size - 1;
- myvprintf_str_XML_simplistic(append_to_xml_buf, &xml_buf, str);
- *xml_buf.cur++ = 0;
- return buf;
-}
-
-
-/* ---------------------------------------------------------------------
elapsed_wallclock_time()
------------------------------------------------------------------ */
@@ -407,6 +366,16 @@
}
/* Send a simple single-part message. */
+UInt VG_(xml_message) ( VgMsgKind kind, const HChar* format, ... )
+{
+ UInt count;
+ va_list vargs;
+ va_start(vargs,format);
+ count = VG_(vmessage) ( kind, format, vargs );
+ va_end(vargs);
+ return count;
+}
+
UInt VG_(message) ( VgMsgKind kind, const HChar* format, ... )
{
UInt count;
|
|
From: Julian S. <js...@ac...> - 2008-07-02 20:34:33
|
On Wednesday 02 July 2008 14:08, Bart Van Assche wrote: > On Tue, Jul 1, 2008 at 10:02 AM, Dirk Mueller <dm...@gm...> wrote: > > VG_(printf) uses "%t" as a "xml-escaped %s" attribute. This does not if > > gcc format checking is enabled, as %t is a nonstandard extension, which > > it doesn't understand. I've asked you some time ago wether it would be > > okay to either map it to "%s" and use a ToXml function for the argument, > > or to use a completely different (non printf-like) function for xml > > output. I guess the latter makes more sense, as line formatting (printf) > > doesn't really make sense with xml output, the stream output can be > > abstracted into something else. I suppose it would be a good thing to move towards separating the XML messaging completely. So, yes, the latter sounds better to me too. > How about the code below ? The patch below defines two variants of the > same function, namely VG_(xml_message)() and VG_(message)(). Format > string checking is disabled for the former and enabled for the latter. > This way the %t format specifier does not trigger a warning when > passed to VG_(xml_message)(). Am I right to understand that VG_(xml_message)() and VG_(message) are functionally identical, or is that not so? J |
|
From: Bart V. A. <bar...@gm...> - 2008-07-03 09:47:17
Attachments:
xmlmsg.patch
|
On Wed, Jul 2, 2008 at 10:26 PM, Julian Seward <js...@ac...> wrote:
> Am I right to understand that VG_(xml_message)() and VG_(message) are
> functionally identical, or is that not so?
You are right -- in the patch I posted the source code of
VG_(message)() and VG_(xml_message)() is identical.
Is it OK if I apply the patch below to the trunk ? This patch contains
the following changes:
- Made %' a synonym of %, (insert thousands separator).
- Add support for multiple flags past %.
- Add support for %#.
- Add VG_(xml_message)().
- Replace some calls of VG_(message)() by calls to VG_(xml_message)().
These changes can also be found on the FORMATCHECK branch. Some of the
above changes were implemented by Dirk. Any errors are my
responsibility of course.
Bart.
--- coregrind/m_debuglog.c 2008-02-11 20:11:19.000000000 +0100
+++ ../valgrind-fmtcheck/coregrind/m_debuglog.c 2008-07-03
09:25:04.000000000 +0200
@@ -446,7 +446,7 @@ static void emit ( HChar* buf, Int n )
#define VG_MSG_LJUSTIFY 4 /* Must justify on the left. */
#define VG_MSG_PAREN 8 /* Parenthesize if present (for %y) */
#define VG_MSG_COMMA 16 /* Add commas to numbers (for %d, %u) */
-
+#define VG_MSG_ALTFORMAT 32 /* Convert the value to alternate format */
/* Copy a string into the buffer. */
static
@@ -647,25 +647,34 @@ VG_(debugLog_vprintf) (
flags = 0;
n_ls = 0;
width = 0; /* length of the field. */
- if (format[i] == '(') {
- flags |= VG_MSG_PAREN;
- i++;
- }
- /* If ',' follows '%', commas will be inserted. */
- if (format[i] == ',') {
- flags |= VG_MSG_COMMA;
- i++;
- }
- /* If '-' follows '%', justify on the left. */
- if (format[i] == '-') {
- flags |= VG_MSG_LJUSTIFY;
- i++;
- }
- /* If '0' follows '%', pads will be inserted. */
- if (format[i] == '0') {
- flags |= VG_MSG_ZJUSTIFY;
+ while (1) {
+ switch (format[i]) {
+ case '(':
+ flags |= VG_MSG_PAREN;
+ break;
+ case ',':
+ case '\'':
+ /* If ',' or '\'' follows '%', commas will be inserted. */
+ flags |= VG_MSG_COMMA;
+ break;
+ case '-':
+ /* If '-' follows '%', justify on the left. */
+ flags |= VG_MSG_LJUSTIFY;
+ break;
+ case '0':
+ /* If '0' follows '%', pads will be inserted. */
+ flags |= VG_MSG_ZJUSTIFY;
+ break;
+ case '#':
+ /* If '#' follows '%', alternative format will be used. */
+ flags |= VG_MSG_ALTFORMAT;
+ break;
+ default:
+ goto parse_fieldwidth;
+ }
i++;
}
+ parse_fieldwidth:
/* Compute the field length. */
while (format[i] >= '0' && format[i] <= '9') {
width *= 10;
@@ -711,6 +720,11 @@ VG_(debugLog_vprintf) (
case 'x': /* %x */
case 'X': /* %X */
caps = toBool(format[i] == 'X');
+ if (flags & VG_MSG_ALTFORMAT) {
+ ret += 2;
+ send('0',send_arg2);
+ send('x',send_arg2);
+ }
if (is_long)
ret += myvprintf_int64(send, send_arg2, flags, 16, width, caps,
(ULong)(va_arg (vargs, ULong)));
--- coregrind/m_errormgr.c 2008-05-02 20:53:42.000000000 +0200
+++ ../valgrind-fmtcheck/coregrind/m_errormgr.c 2008-07-03
09:25:43.000000000 +0200
@@ -709,7 +709,7 @@ static Bool show_used_suppressions ( voi
continue;
any_supp = True;
if (VG_(clo_xml)) {
- VG_(message)(Vg_DebugMsg,
+ VG_(xml_message)(Vg_DebugMsg,
" <pair>\n"
" <count>%d</count>\n"
" <name>%t</name>\n"
--- coregrind/m_libcprint.c 2008-04-17 18:57:42.000000000 +0200
+++ ../valgrind-fmtcheck/coregrind/m_libcprint.c 2008-07-03
10:45:07.000000000 +0200
@@ -365,6 +365,17 @@ UInt VG_(vmessage) ( VgMsgKind kind, con
return count;
}
+/* Send a single-part XML message. */
+UInt VG_(xml_message) ( VgMsgKind kind, const HChar* format, ... )
+{
+ UInt count;
+ va_list vargs;
+ va_start(vargs,format);
+ count = VG_(vmessage) ( kind, format, vargs );
+ va_end(vargs);
+ return count;
+}
+
/* Send a simple single-part message. */
UInt VG_(message) ( VgMsgKind kind, const HChar* format, ... )
{
--- coregrind/m_main.c 2008-06-06 19:04:59.000000000 +0200
+++ ../valgrind-fmtcheck/coregrind/m_main.c 2008-07-03 09:29:50.000000000 +0200
@@ -747,7 +747,7 @@ static void print_file_vars(Char* format
i++;
}
- VG_(message)(Vg_UserMsg, "<logfilequalifier> <var>%t</var> "
+ VG_(xml_message)(Vg_UserMsg, "<logfilequalifier> <var>%t</var> "
"<value>%t</value> </logfilequalifier>",
qualname,qual);
format[i] = '}';
@@ -853,7 +853,7 @@ static void print_preamble(Bool logging_
VG_(message)(Vg_UserMsg, "");
VG_(message)(Vg_UserMsg, "<pid>%d</pid>", VG_(getpid)());
VG_(message)(Vg_UserMsg, "<ppid>%d</ppid>", VG_(getppid)());
- VG_(message)(Vg_UserMsg, "<tool>%t</tool>", toolname);
+ VG_(xml_message)(Vg_UserMsg, "<tool>%t</tool>", toolname);
if (VG_(clo_log_name))
print_file_vars(VG_(clo_log_name));
if (VG_(clo_xml_user_comment)) {
@@ -868,26 +868,26 @@ static void print_preamble(Bool logging_
VG_(message)(Vg_UserMsg, " <vargv>");
if (VG_(name_of_launcher))
- VG_(message)(Vg_UserMsg, " <exe>%t</exe>",
- VG_(name_of_launcher));
+ VG_(xml_message)(Vg_UserMsg, " <exe>%t</exe>",
+ VG_(name_of_launcher));
else
- VG_(message)(Vg_UserMsg, " <exe>%t</exe>",
- "(launcher name unknown)");
+ VG_(xml_message)(Vg_UserMsg, " <exe>%t</exe>",
+ "(launcher name unknown)");
for (i = 0; i < VG_(sizeXA)( VG_(args_for_valgrind) ); i++) {
- VG_(message)(Vg_UserMsg,
- " <arg>%t</arg>",
- * (HChar**) VG_(indexXA)( VG_(args_for_valgrind), i ));
+ VG_(xml_message)(Vg_UserMsg,
+ " <arg>%t</arg>",
+ * (HChar**) VG_(indexXA)(
VG_(args_for_valgrind), i ));
}
VG_(message)(Vg_UserMsg, " </vargv>");
VG_(message)(Vg_UserMsg, " <argv>");
if (VG_(args_the_exename))
- VG_(message)(Vg_UserMsg, " <exe>%t</exe>",
- VG_(args_the_exename));
+ VG_(xml_message)(Vg_UserMsg, " <exe>%t</exe>",
+ VG_(args_the_exename));
for (i = 0; i < VG_(sizeXA)( VG_(args_for_client) ); i++) {
- VG_(message)(Vg_UserMsg,
- " <arg>%t</arg>",
- * (HChar**) VG_(indexXA)( VG_(args_for_client), i ));
+ VG_(xml_message)(Vg_UserMsg,
+ " <arg>%t</arg>",
+ * (HChar**) VG_(indexXA)( VG_(args_for_client), i ));
}
VG_(message)(Vg_UserMsg, " </argv>");
@@ -1934,11 +1934,12 @@ Int valgrind_main ( Int argc, HChar **ar
if (VG_(clo_xml)) {
HChar buf[50];
VG_(elapsed_wallclock_time)(buf);
- VG_(message)(Vg_UserMsg, "<status>\n"
- " <state>RUNNING</state>\n"
- " <time>%t</time>\n"
- "</status>",
- buf);
+ VG_(xml_message)(Vg_UserMsg,
+ "<status>\n"
+ " <state>RUNNING</state>\n"
+ " <time>%t</time>\n"
+ "</status>",
+ buf);
VG_(message)(Vg_UserMsg, "");
}
@@ -2045,11 +2046,12 @@ void shutdown_actions_NORETURN( ThreadId
VG_(message)(Vg_UserMsg, "");
}
VG_(elapsed_wallclock_time)(buf);
- VG_(message)(Vg_UserMsg, "<status>\n"
- " <state>FINISHED</state>\n"
- " <time>%t</time>\n"
- "</status>",
- buf);
+ VG_(xml_message)(Vg_UserMsg,
+ "<status>\n"
+ " <state>FINISHED</state>\n"
+ " <time>%t</time>\n"
+ "</status>",
+ buf);
VG_(message)(Vg_UserMsg, "");
}
--- include/pub_tool_libcprint.h 2008-06-26 09:26:45.000000000 +0200
+++ ../valgrind-fmtcheck/include/pub_tool_libcprint.h 2008-07-03
09:13:41.000000000 +0200
@@ -85,10 +84,22 @@ typedef
}
VgMsgKind;
-/* Send a single-part message. Appends a newline. */
-extern UInt VG_(message) ( VgMsgKind kind, const HChar* format,
... ) PRINTF_CHECK(2, 3);
+/* Send a single-part XML message. Appends a newline. The format
+ specification may contain any ISO C format specifier or %t.
+ No attempt is made to let the compiler verify consistency of the
+ format string and the argument list. */
+extern UInt VG_(xml_message)( VgMsgKind kind, const HChar* format, ... );
+/* Send a single-part message. Appends a newline. The format
+ specification may contain any ISO C format specifier. The gcc compiler
+ will verify consistency of the format string and the argument list. */
+extern UInt VG_(message)( VgMsgKind kind, const HChar* format, ... )
+ PRINTF_CHECK(2, 3);
+
+extern UInt VG_(vmessage)( VgMsgKind kind, const HChar* format, va_list vargs )
+ PRINTF_CHECK(2, 0);
+
+
-extern UInt VG_(vmessage) ( VgMsgKind kind, const HChar* format,
va_list vargs ) PRINTF_CHECK(2, 0);
#endif // __PUB_TOOL_LIBCPRINT_H
/*--------------------------------------------------------------------*/
--- memcheck/mc_errors.c 2008-05-02 20:53:42.000000000 +0200
+++ ../valgrind-fmtcheck/memcheck/mc_errors.c 2008-07-02
14:01:31.000000000 +0200
@@ -315,14 +315,14 @@ static void mc_pp_AddrInfo ( Addr a, Add
}
case Addr_DataSym:
- VG_(message)(Vg_UserMsg,
- "%sAddress 0x%llx is %llu bytes "
- "inside data symbol \"%t\"%s",
- xpre,
- (ULong)a,
- (ULong)ai->Addr.DataSym.offset,
- ai->Addr.DataSym.name,
- xpost);
+ VG_(xml_message)(Vg_UserMsg,
+ "%sAddress 0x%llx is %llu bytes "
+ "inside data symbol \"%t\"%s",
+ xpre,
+ (ULong)a,
+ (ULong)ai->Addr.DataSym.offset,
+ ai->Addr.DataSym.name,
+ xpost);
break;
case Addr_Variable:
@@ -335,13 +335,13 @@ static void mc_pp_AddrInfo ( Addr a, Add
break;
case Addr_SectKind:
- VG_(message)(Vg_UserMsg,
- "%sAddress 0x%llx is in the %t segment of %t%s",
- xpre,
- (ULong)a,
- VG_(pp_SectKind)(ai->Addr.SectKind.kind),
- ai->Addr.SectKind.objname,
- xpost);
+ VG_(xml_message)(Vg_UserMsg,
+ "%sAddress 0x%llx is in the %t segment of %t%s",
+ xpre,
+ (ULong)a,
+ VG_(pp_SectKind)(ai->Addr.SectKind.kind),
+ ai->Addr.SectKind.objname,
+ xpost);
break;
default:
@@ -568,8 +568,8 @@ void MC_(pp_Error) ( Error* err )
LossRecord* l = extra->Err.Leak.lossRecord;
if (VG_(clo_xml)) {
- VG_(message)(Vg_UserMsg, " <kind>%t</kind>",
- xml_leak_kind(l->loss_mode));
+ VG_(xml_message)(Vg_UserMsg, " <kind>%t</kind>",
+ xml_leak_kind(l->loss_mode));
} else {
VG_(message)(Vg_UserMsg, "");
}
|
|
From: Julian S. <js...@ac...> - 2008-07-04 05:55:14
|
On Thursday 03 July 2008 11:47, Bart Van Assche wrote: > On Wed, Jul 2, 2008 at 10:26 PM, Julian Seward <js...@ac...> wrote: > > Am I right to understand that VG_(xml_message)() and VG_(message) are > > functionally identical, or is that not so? > > You are right -- in the patch I posted the source code of > VG_(message)() and VG_(xml_message)() is identical. > > Is it OK if I apply the patch below to the trunk ? This patch contains > the following changes: Looks OK in general. However, if the patch is logically part of the format checking changes, why do you want to apply it directly to the trunk instead of via any eventual merge of the FORMATCHECK branch? > - Made %' a synonym of %, (insert thousands separator). We need to check if gcc-3.0.4 -- the oldest supported gcc now -- can deal with that. man 3 printf says "Note that many versions of gcc(1) cannot parse this option and will issue a warning." I'll build gcc-3.0.4 and check. > - Add support for multiple flags past %. > - Add support for %#. > - Add VG_(xml_message)(). > - Replace some calls of VG_(message)() by calls to VG_(xml_message)(). Ok; except that since VG_(xml_message) is exactly the same as VG_(message) and nothing really to do with xml, I'd prefer to call it VG_(message_no_f_c) to make the difference clearer (no-format-check). J |
|
From: Bart V. A. <bar...@gm...> - 2008-07-04 07:29:47
|
On Fri, Jul 4, 2008 at 7:47 AM, Julian Seward <js...@ac...> wrote: > On Thursday 03 July 2008 11:47, Bart Van Assche wrote: > > Is it OK if I apply the patch below to the trunk ? This patch contains > > the following changes: > > Looks OK in general. However, if the patch is logically part of the > format checking changes, why do you want to apply it directly to the > trunk instead of via any eventual merge of the FORMATCHECK branch? Merging the whole FORMATCHECK branch at once to the trunk is also fine for me. Who should do this, and when should this happen ? > > - Made %' a synonym of %, (insert thousands separator). > > We need to check if gcc-3.0.4 -- the oldest supported gcc now -- > can deal with that. man 3 printf says "Note that many versions of > gcc(1) cannot parse this option and will issue a warning." I'll > build gcc-3.0.4 and check. Do we really support compilation of Valgrind with gcc 3.0 and 3.1 ? These two compiler versions are not supported for compiling the Linux kernel because these two versions do not compile the kernel code correctly. > > - Add support for multiple flags past %. > > - Add support for %#. > > - Add VG_(xml_message)(). > > - Replace some calls of VG_(message)() by calls to VG_(xml_message)(). > > Ok; except that since VG_(xml_message) is exactly the same as VG_(message) > and nothing really to do with xml, I'd prefer to call it VG_(message_no_f_c) > to make the difference clearer (no-format-check). By this time VG_(xml_message)() has been renamed to VG_(message_no_f_c)(). Bart. |
|
From: Julian S. <js...@ac...> - 2008-07-04 15:03:32
|
On Friday 04 July 2008 09:29, Bart Van Assche wrote: > Merging the whole FORMATCHECK branch at once to the trunk is also fine > for me. Who should do this, and when should this happen ? First, get it to the point where you and Dirk consider it to be complete/working/ok for testing (maybe it already is at that point, but you need to make this clear). Then mail the list to say so, so as to give others a chance to test it. > Do we really support compilation of Valgrind with gcc 3.0 and 3.1 ? > These two compiler versions are not supported for compiling the Linux > kernel because these two versions do not compile the kernel code > correctly. If we can support 3.0 and 3.1 without difficulty, I don't see why we shouldn't. I've been trying to build 3.0.4 and then build FORMATCHECK with it, but so far failed for unrelated issues. I'll keep trying over the weekend. J |
|
From: Bart V. A. <bar...@gm...> - 2008-07-05 08:37:01
|
On Fri, Jul 4, 2008 at 4:55 PM, Julian Seward <js...@ac...> wrote: > If we can support 3.0 and 3.1 without difficulty, I don't see why > we shouldn't. I've been trying to build 3.0.4 and then build > FORMATCHECK with it, but so far failed for unrelated issues. I'll > keep trying over the weekend. I have tried to build gcc 3.0.4 myself, and noted that the gcc 3.0.4 source code uses lvalue casts. These have been deprecated in gcc 3.4 and have been removed in gcc 4.0. See also http://gcc.gnu.org/gcc-3.4/changes.html. Bart. |