[xiph-commits] r17071 - trunk/vorbis-tools/oggenc

[giles at svn.xiph.org]

Author: giles
Date: 2010-03-25 22:00:56 -0700 (Thu, 25 Mar 2010)
New Revision: 17071

Modified:
   trunk/vorbis-tools/oggenc/oggenc.c
   trunk/vorbis-tools/oggenc/platform.h
Log:
Use strcmp instead of pointer comparison against string literals.

The oggenc option parsing set some filter strings to a default,
then checked later settings from the command line against that
default to decide whether to warn about multiple settings of
the same value. This works fine when the default is 'NULL' but
unforutnately in the case where the default is a string literal
the results of the pointer comparison is undefined: the compiler
will generally collapse the two instances of the same #define'd
literal, but it is not required to do so. And if fact, the clang
compiler warns about this.

Instead, we use strcmp() to compare the values of the two different
pointers, which will be identical if they both point to the literal.

This works in the case of DEFAULT_NAMEFMT_REMOVE, but generated a
new warning on DEFAULT_NAMEFMT_REPLACE which was NULL. Clang also
warns (incorrectly) on  the code:

#define bar NULL
if (foo && bar && strcmp(foo, bar));

so instead, we use the empty string for an empty default. This lets
us preserve the identical code paths for the two options, and have
things still work if the defaults are changed later.

The logic is still a little odd as it won't warn about successive
identical settings, but perhaps that's a feature.


Modified: trunk/vorbis-tools/oggenc/oggenc.c
===================================================================
--- trunk/vorbis-tools/oggenc/oggenc.c	2010-03-26 04:50:44 UTC (rev 17070)
+++ trunk/vorbis-tools/oggenc/oggenc.c	2010-03-26 05:00:56 UTC (rev 17071)
@@ -874,8 +874,8 @@
                 opt->namefmt = strdup(optarg);
                 break;
             case 'X':
-                if(opt->namefmt_remove && opt->namefmt_remove != 
-                        DEFAULT_NAMEFMT_REMOVE)
+                if(opt->namefmt_remove &&
+                        strcmp(opt->namefmt_remove, DEFAULT_NAMEFMT_REMOVE))
                 {
                     fprintf(stderr, _("WARNING: Multiple name format filters specified, using final\n"));
                     free(opt->namefmt_remove);
@@ -883,8 +883,8 @@
                 opt->namefmt_remove = strdup(optarg);
                 break;
             case 'P':
-                if(opt->namefmt_replace && opt->namefmt_replace != 
-                        DEFAULT_NAMEFMT_REPLACE)
+                if(opt->namefmt_replace &&
+                        strcmp(opt->namefmt_replace, DEFAULT_NAMEFMT_REPLACE))
                 {
                     fprintf(stderr, _("WARNING: Multiple name format filter replacements specified, using final\n"));
                     free(opt->namefmt_replace);

Modified: trunk/vorbis-tools/oggenc/platform.h
===================================================================
--- trunk/vorbis-tools/oggenc/platform.h	2010-03-26 04:50:44 UTC (rev 17070)
+++ trunk/vorbis-tools/oggenc/platform.h	2010-03-26 05:00:56 UTC (rev 17071)
@@ -19,13 +19,13 @@
 void setbinmode(FILE *);
 
 #define DEFAULT_NAMEFMT_REMOVE "/\\:<>|"
-#define DEFAULT_NAMEFMT_REPLACE NULL
+#define DEFAULT_NAMEFMT_REPLACE ""
 
 #else /* Unix, mostly */
 
 #define setbinmode(x) {}
 #define DEFAULT_NAMEFMT_REMOVE "/"
-#define DEFAULT_NAMEFMT_REPLACE NULL
+#define DEFAULT_NAMEFMT_REPLACE ""
 
 #endif