You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> on 2007/12/26 20:01:20 UTC

Re: [PATCH] - Fix a compiler warning on OSX

2007-11-22 20:14:33 David Glasser napisał(a):
> On Nov 22, 2007 1:09 PM, Arfrever Frehtes Taifersar Arahesis
> <ar...@gmail.com> wrote:
> > 2007-11-20 20:12:38 Karl Fogel napisał(a):
> > > "Mark Phippard" <ma...@gmail.com> writes:
> > > > I noticed this in my output.  I report it only because a similar
> > > > warning in the mergeinfo.c code was fixed last week, therefore I
> > > > assume people think it is a good idea to fix these.
> > > >
> > > > subversion/libsvn_subr/xml.c: In function 'svn_xml_parse':
> > > > subversion/libsvn_subr/xml.c:402: warning: format '%d' expects type
> > > > 'int', but argument 5 has type 'XML_Size'
> > > >
> > > > Changing the format specifier to %ld makes the warning go away.  I
> > > > imagine an XML file is not likely to have a line number that is bigger
> > > > than the size of int, and I was not sure if there are some
> > > > platform-specific size differences, so I did not just blindly commit
> > > > it.  Also, according to blame it looks like the code has been around a
> > > > while.
> > > >
> > > > I have attached a patch with log message.  If this is something we
> > > > should fix then let me know and I can commit it.
> > >
> > > What version of expat do you have?  I have 1.95.8; you can look in
> > > /usr/include/expat.h for this:
> > >
> > >    #define XML_MAJOR_VERSION 1
> > >    #define XML_MINOR_VERSION 95
> > >    #define XML_MICRO_VERSION 8
> > >
> > > Also in expat.h, I see this:
> > >
> > >    XMLPARSEAPI(int) XML_GetCurrentLineNumber(XML_Parser parser);
> >
> > I have in /usr/include/expat.h:
> >   #define XML_MAJOR_VERSION 2
> >   #define XML_MINOR_VERSION 0
> >   #define XML_MICRO_VERSION 1
> >   ...
> >   XMLPARSEAPI(XML_Size) XML_GetCurrentLineNumber(XML_Parser parser);
> 
> Yeah, so expat 2 returns XML_Size (which is roughly some sort of
> long), whereas expat 1 returns int.
> 
> We could use some preprocessor trickery involving XML_MAJOR_VERSION to
> calculate the proper format string.

[[[
* subversion/libsvn_subr/xml.c
  (svn_xml_parse): When creating error messages, use format specifier
  conditionalized by the version of Expat.

Patch by: markphip
          arfrever
]]]

-- 
Arfrever Frehtes Taifersar Arahesis

Re: [PATCH] Fix compilation warning with Expat 2

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-01-27 01:09:35 Julian Foad napisał(a):
> Hi Arfrever.
> 
> Thanks for the report and the patch and your persistence. I agree your patch is 
> correct. However, I committed the following alternative fix in r29057, which 
> avoids an explicit dependency on particular versions of Expat, and has only one 
> copy of the error message for translators to maintain.
> 
> I tested it by compiling with the function redefined manually to return various 
> types, and looking for the warning to go away with all reasonable integer 
> types. (I have Expat v1 installed.)
> 
> Let me know if there is any problem with this solution, of course.

Thanks. It works correctly.
(Copyright years are still 2000-2006 :) .)

-- 
Arfrever Frehtes Taifersar Arahesis

Re: [PATCH] Fix compilation warning with Expat 2

Posted by Julian Foad <ju...@btopenworld.com>.
Hi Arfrever.

Thanks for the report and the patch and your persistence. I agree your patch is 
correct. However, I committed the following alternative fix in r29057, which 
avoids an explicit dependency on particular versions of Expat, and has only one 
copy of the error message for translators to maintain.

I tested it by compiling with the function redefined manually to return various 
types, and looking for the warning to go away with all reasonable integer 
types. (I have Expat v1 installed.)

Let me know if there is any problem with this solution, of course.

[[[
Index: subversion/libsvn_subr/xml.c
===================================================================
--- subversion/libsvn_subr/xml.c        (revision 29055)
+++ subversion/libsvn_subr/xml.c        (working copy)
@@ -395,11 +395,13 @@
    /* If expat choked internally, return its error. */
    if (! success)
      {
+      /* Line num is "int" in Expat v1, "long" in v2; hide the difference. */
+      long line = XML_GetCurrentLineNumber(svn_parser->parser);
+
        err = svn_error_createf
          (SVN_ERR_XML_MALFORMED, NULL,
-         _("Malformed XML: %s at line %d"),
-         XML_ErrorString(XML_GetErrorCode(svn_parser->parser)),
-         XML_GetCurrentLineNumber(svn_parser->parser));
+         _("Malformed XML: %s at line %ld"),
+         XML_ErrorString(XML_GetErrorCode(svn_parser->parser)), line);

        /* Kill all parsers and return the expat error */
        svn_xml_free_parser(svn_parser);
]]]

Regards,
- Julian


Arfrever Frehtes Taifersar Arahesis wrote:
> [[[
> Fix compilation warning with Expat 2.
> 
> * subversion/libsvn_subr/xml.c
>   (svn_xml_parse): When creating error messages, use format specifier
>    conditionalized on the version of Expat.
> 
> Patch by: markphip
>           arfrever
> ]]]
> 
> The patch is here:
> http://svn.haxx.se/dev/archive-2007-12/0766.shtml

For quick reference:
> Index: subversion/libsvn_subr/xml.c
> ===================================================================
> --- subversion/libsvn_subr/xml.c        (revision 29055)
> +++ subversion/libsvn_subr/xml.c        (working copy)
> @@ -397,7 +397,11 @@
>      {
>        err = svn_error_createf
>          (SVN_ERR_XML_MALFORMED, NULL,
> +#if XML_MAJOR_VERSION >= 2
> +         _("Malformed XML: %s at line %ld"),
> +#else
>           _("Malformed XML: %s at line %d"),
> +#endif
>           XML_ErrorString(XML_GetErrorCode(svn_parser->parser)),
>           XML_GetCurrentLineNumber(svn_parser->parser));
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] Fix compilation warning with Expat 2

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
[[[
Fix compilation warning with Expat 2.

* subversion/libsvn_subr/xml.c
  (svn_xml_parse): When creating error messages, use format specifier
   conditionalized on the version of Expat.

Patch by: markphip
          arfrever
]]]

The patch is here:
http://svn.haxx.se/dev/archive-2007-12/0766.shtml

-- 
Arfrever Frehtes Taifersar Arahesis

[PATCH] Support Expat 2 (Was: Fix a compiler warning on OSX)

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2007-12-26 21:00:57 Arfrever Frehtes Taifersar Arahesis napisał(a):
> 2007-11-22 20:14:33 David Glasser napisał(a):
> > On Nov 22, 2007 1:09 PM, Arfrever Frehtes Taifersar Arahesis
> > <ar...@gmail.com> wrote:
> > > 2007-11-20 20:12:38 Karl Fogel napisał(a):
> > > > "Mark Phippard" <ma...@gmail.com> writes:
> > > > > I noticed this in my output.  I report it only because a similar
> > > > > warning in the mergeinfo.c code was fixed last week, therefore I
> > > > > assume people think it is a good idea to fix these.
> > > > >
> > > > > subversion/libsvn_subr/xml.c: In function 'svn_xml_parse':
> > > > > subversion/libsvn_subr/xml.c:402: warning: format '%d' expects type
> > > > > 'int', but argument 5 has type 'XML_Size'
> > > > >
> > > > > Changing the format specifier to %ld makes the warning go away.  I
> > > > > imagine an XML file is not likely to have a line number that is bigger
> > > > > than the size of int, and I was not sure if there are some
> > > > > platform-specific size differences, so I did not just blindly commit
> > > > > it.  Also, according to blame it looks like the code has been around a
> > > > > while.
> > > > >
> > > > > I have attached a patch with log message.  If this is something we
> > > > > should fix then let me know and I can commit it.
> > > >
> > > > What version of expat do you have?  I have 1.95.8; you can look in
> > > > /usr/include/expat.h for this:
> > > >
> > > >    #define XML_MAJOR_VERSION 1
> > > >    #define XML_MINOR_VERSION 95
> > > >    #define XML_MICRO_VERSION 8
> > > >
> > > > Also in expat.h, I see this:
> > > >
> > > >    XMLPARSEAPI(int) XML_GetCurrentLineNumber(XML_Parser parser);
> > >
> > > I have in /usr/include/expat.h:
> > >   #define XML_MAJOR_VERSION 2
> > >   #define XML_MINOR_VERSION 0
> > >   #define XML_MICRO_VERSION 1
> > >   ...
> > >   XMLPARSEAPI(XML_Size) XML_GetCurrentLineNumber(XML_Parser parser);
> > 
> > Yeah, so expat 2 returns XML_Size (which is roughly some sort of
> > long), whereas expat 1 returns int.
> > 
> > We could use some preprocessor trickery involving XML_MAJOR_VERSION to
> > calculate the proper format string.

[[[
* subversion/libsvn_subr/xml.c
  (svn_xml_parse): When creating error messages, use format specifier
  conditionalized on the version of Expat.

Patch by: markphip
          arfrever
]]]

Can I commit this patch?
http://svn.haxx.se/dev/archive-2007-12/0766.shtml

-- 
Arfrever Frehtes Taifersar Arahesis