[MPlayer-dev-eng] Re: [MPlayer-cvslog] r21218 - trunk/stream/freesdp/parser.c

[Reimar Doeffinger]

Hello,
On Mon, Nov 27, 2006 at 10:19:21AM +0100, Reimar Doeffinger wrote:
> On Mon, Nov 27, 2006 at 03:10:18AM -0500, Rich Felker wrote:
> > On Sat, Nov 25, 2006 at 02:39:22PM +0100, reimar wrote:
> > > Author: reimar
> > > Date: Sat Nov 25 14:39:21 2006
> > > New Revision: 21218
> > > 
> > > Modified:
> > >    trunk/stream/freesdp/parser.c
> > > 
> > > Log:
> > > spurious () like in ({code;}) probably is not valid C, icc 9, definitely
> > > will not compile it, and whatever it is supposed to be good for it
> > > does not seem to be needed.
> > > 
> > > 
> > > Modified: trunk/stream/freesdp/parser.c
> > > ==============================================================================
> > > --- trunk/stream/freesdp/parser.c	(original)
> > > +++ trunk/stream/freesdp/parser.c	Sat Nov 25 14:39:21 2006
> > > @@ -44,7 +44,7 @@
> > >   * (not followed by a '\n') is found, returns
> > >   */
> > >  #define NEXT_LINE(c)                                                \
> > > -({                                                                  \
> > > +{                                                                   \
> > >   while ((*(c) != '\0') && (*(c) != '\r') && (*(c) != '\n')) {       \
> > >      (c)++;                                                          \
> > >   }                                                                  \
> > > @@ -58,7 +58,7 @@
> > >         return FSDPE_ILLEGAL_CHARACTER;                              \
> > >     }                                                                \
> > >   }                                                                  \
> > > -})
> > > +}
> > 
> > don't use { ... } but instead do { ... } while(0). the former will 
> > misbehave if you do something like if (foo) MACRO(); else ...
> 
> It's not necessary and ugly. Making it a function is probably a
> much better idea...
> It could be simplified by using strcspn, too, I'd just need something to
> test it.
> But actually since it is imported code I'm not sure that making more
> than the absolutely necessary changes is a good idea.

Since google does not indicate that anything like "FreeSDP" even exists
(at least as an independent project), I could suggest attached patch.

Greetings,
Reimar D?ffinger
-------------- next part --------------
Index: stream/freesdp/parser.c
===================================================================
--- stream/freesdp/parser.c	(revision 21307)
+++ stream/freesdp/parser.c	(working copy)
@@ -36,6 +36,21 @@
 #include "parserpriv.h"
 
 /**
+ * \brief find the start of the next line
+ * \param c pointer to current position in string
+ * \return pointer to start of next line or NULL if illegal (i.e.
+ *         a '\r' is not followed by a '\n'
+ */
+static const char *next_line(const char *c) {
+  c += strcspn(c, "\n\r");
+  if (*c == 0) return c;
+  if (*c == '\r') c++;
+  if (*c == '\n')
+    return c + 1;
+  return NULL;
+}
+
+/**
  * Moves the <code>c<code> pointer up to the beginning of the next
  * line.
  *
@@ -43,22 +58,7 @@
  * @retval FSDPE_ILLEGAL_CHARACTER, when an illegal '\r' character
  * (not followed by a '\n') is found, returns
  */
-#define NEXT_LINE(c)                                                \
-{                                                                   \
- while ((*(c) != '\0') && (*(c) != '\r') && (*(c) != '\n')) {       \
-    (c)++;                                                          \
- }                                                                  \
- if (*(c) == '\n') {                                                \
-    (c)++;                                                          \
- } else if (*(c) == '\r') {                                         \
-    (c)++;                                                          \
-    if (*(c) == '\n') {                                             \
-       (c)++;                                                       \
-    } else {                                                        \
-       return FSDPE_ILLEGAL_CHARACTER;                              \
-   }                                                                \
- }                                                                  \
-}
+#define NEXT_LINE(c) do { if (!(c = next_line(c))) return FSDPE_ILLEGAL_CHARACTER; } while (0);
 
 fsdp_error_t
 fsdp_parse (const char *text_description, fsdp_description_t * dsc)