Handle duplicate comments, with unit test

Squash multiple instances of the same comment name=value,
don't hang on scanning comment list by value comparison.
See http://lists.xiph.org/pipermail/ogg-dev/2008-September/001196.html

src/liboggz/oggz_comments.c

@@ -354,7 +354,30 @@ oggz_comment_next_byname (OGGZ * oggz, long serialno,
   return NULL;
 }
 
-#define _oggz_comment_add(f,c) oggz_vector_insert_p ((f)->comments, (c))
+static OggzComment *
+_oggz_comment_add_byname (oggz_stream_t * stream, const char * name, const char * value)
+{
+  OggzComment * comment, * new_comment;
+  int i;
+
+  /* Check that the same name=value pair is not already present */
+  for (i = 0; i < oggz_vector_size (stream->comments); i++) {
+    comment = (OggzComment *) oggz_vector_nth_p (stream->comments, i);
+    if (comment->name && !strcasecmp (name, comment->name)) {
+      if (comment->value == NULL) {
+        if (value == NULL) return comment;
+      } else if (!strcmp (value, comment->value)) {
+        return comment;
+      }
+    }
+  }
+
+  /* Allocate new comment and insert it */
+  if ((new_comment = oggz_comment_new (name, value)) == NULL)
+    return NULL;
+
+  return oggz_vector_insert_p (stream->comments, new_comment);
+}
 
 int
 oggz_comment_add (OGGZ * oggz, long serialno, const OggzComment * comment)
@@ -376,10 +399,7 @@ oggz_comment_add (OGGZ * oggz, long serialno, const OggzComment * comment)
       if (!oggz_comment_validate_byname (comment->name, comment->value))
         return OGGZ_ERR_COMMENT_INVALID;
 
-      if ((new_comment = oggz_comment_new (comment->name, comment->value)) == NULL)
-        return OGGZ_ERR_OUT_OF_MEMORY;
-
-      if (_oggz_comment_add (stream, new_comment) == NULL)
+      if (_oggz_comment_add_byname (stream, comment->name, comment->value) == NULL)
         return OGGZ_ERR_OUT_OF_MEMORY;
 
       return 0;
@@ -412,10 +432,7 @@ oggz_comment_add_byname (OGGZ * oggz, long serialno,
       if (!oggz_comment_validate_byname (name, value))
         return OGGZ_ERR_COMMENT_INVALID;
 
-      if ((new_comment = oggz_comment_new (name, value)) == NULL)
-        return OGGZ_ERR_OUT_OF_MEMORY;
-
-      if (_oggz_comment_add (stream, new_comment) == NULL)
+      if (_oggz_comment_add_byname (stream, name, value) == NULL)
         return OGGZ_ERR_OUT_OF_MEMORY;
 
       return 0;
@@ -609,10 +626,7 @@ oggz_comments_decode (OGGZ * oggz, long serialno,
          printf ("oggz_comments_decode: %s -> %s (length %d)\n",
          name, nvalue, n);
 #endif
-         if ((comment = oggz_comment_new (name, nvalue)) == NULL)
-           return OGGZ_ERR_OUT_OF_MEMORY;
-
-         if (_oggz_comment_add (stream, comment) == NULL)
+         if (_oggz_comment_add_byname (stream, name, nvalue) == NULL)
            return OGGZ_ERR_OUT_OF_MEMORY;
 
          oggz_free (nvalue);
@@ -620,10 +634,7 @@ oggz_comments_decode (OGGZ * oggz, long serialno,
          if ((nvalue = oggz_strdup_len (name, len)) == NULL)
            return OGGZ_ERR_OUT_OF_MEMORY;
 
-         if ((comment = oggz_comment_new (nvalue, NULL)) == NULL)
-           return OGGZ_ERR_OUT_OF_MEMORY;
-
-         if (_oggz_comment_add (stream, comment) == NULL)
+         if (_oggz_comment_add_byname (stream, nvalue, NULL) == NULL)
            return OGGZ_ERR_OUT_OF_MEMORY;
 
          oggz_free (nvalue);

src/tests/comment-test.c

@@ -47,6 +47,7 @@
 #define COPYRIGHT "Copyright (C) 2004. Some Rights Reserved."
 #define LICENSE "Creative Commons Attribute Share-Alike v1.0"
 #define COMMENT "Unstructured comments are evil."
+#define PERFORMER "Jack Mackerel"
 
 static OGGZ * oggz;
 
@@ -268,6 +269,16 @@ main (int argc, char * argv[])
   oggz_packet_destroy(op);
 
 
+  INFO ("+ Adding duplicate tag");
+  err = oggz_comment_add_byname (oggz, serialno, "PERFORMER", PERFORMER);
+  if (err < 0) FAIL ("Operation failed");
+  err = oggz_comment_add_byname (oggz, serialno, "PERFORMER", PERFORMER);
+  if (err < 0) FAIL ("Operation failed");
+  INFO ("+ Scanning comments (should not hang on duplicate)");
+  for (comment = oggz_comment_first (oggz, serialno);
+       comment != NULL;
+       comment = oggz_comment_next (oggz, serialno, comment));
+
   INFO ("Closing OGGZ (writer)");
   oggz_close (oggz);
 #endif /* OGGZ_CONFIG_WRITE */