[MPlayer-dev-eng] Re: [TEST PATCH] How to get rid of all sscanf() or new face of config :)

[Andriy N. Gritsenko]

    Hi, Ivan!

Sometime (on Sunday, March 30 at  1:28) I've received something...
>Thank you for the patch. This time I got it working. And indeed,
>it does what I expected, I'm able to set the list of channels in
>the config file like this:

>tv:channels=e9-NL1,e11-NL2,28-RTL4,63-RTL5,31-SBS6,21-Yorin,E12-Belga1,SE11-Belga2,E8-ARD,SE9-ZDF,24-BBC

>But something isn't clear to me. Is this the result of your patch,
>or was this possiblity there even before? I'm afraid it was, since
>I get these things even without the patch. These things also include
>one inconvenience and one bug.

>1. if I set a flag paramater in the config file without an actual
>   value, I get a segmentation fault, e.g.

>tv:mjpeg

>gives SIGSEGV:
[.......]
>whereas

>tv:mjpeg=1

>works as it should.

    It's not bug but feature, each parameter in config file have to have
a value. Anyway it have not generate SIGSEGV. Fixed, see attached patch.

>2. when I append one more item to the list of channels above, it
>   makes mplayer exit with the following message:

>"too long parameter"

    There was a message about that already. You have to increase value of
MAX_PARAM_LEN in parser-cfg.c - it has 100 by default. BTW, I hope it
will be changed in CVS sometime also. :)

>There's also a difference. I also have in my config file the
>following:

>vo=mga,xv,jpeg

>With your patch, I'm not able to override this setting, and it
>always gives me mga (when the mga driver is loaded). This is
>something I don't like, because without your patch, it did
>work, and I need this functionality.

    Fixed in attached patch. Works now. :)

    With best wishes.
    Andriy.
-------------- next part --------------
diff -udpr MPlayer-20030325.old/m_config.c MPlayer-20030325/m_config.c
--- MPlayer-20030325.old/m_config.c	Wed Mar 26 13:59:13 2003
+++ MPlayer-20030325/m_config.c	Sun Mar 30 22:02:43 2003
@@ -313,6 +313,8 @@ static int m_config_parse_co(m_config_op
 
     // Parse the child options to parameters
     r = m_option_parse(opt,name,param,&lst,M_PREPARSE);
+    // Some cleanup before work, ignore result
+    if (r >= 0 && set) m_option_parse(opt,name,NULL,&opt->priv,mode);
     // fill current slot if there was a parameter
     if (r >= 0 && lst) {
       for (i = 0; lst[2*i]; i++) {
@@ -347,7 +349,7 @@ static int m_config_parse_co(m_config_op
 	      r = M_OPT_INVALID;
 	    } else if(sr < 0)
 	      r = sr;
-	    if (r >= 0)
+	    if (r >= 0 && set)
 	      // option accessed so set opt->priv if need
 	      m_option_parse(opt,name,sc->opt->name,&opt->priv,mode);
 	  }
diff -udpr MPlayer-20030325.old/m_option.c MPlayer-20030325/m_option.c
--- MPlayer-20030325.old/m_option.c	Wed Mar 26 14:02:31 2003
+++ MPlayer-20030325/m_option.c	Sun Mar 30 22:04:12 2003
@@ -30,7 +30,7 @@ m_option_type_t m_option_type_dummy = {
   "Separator",
   "",
   0,
-  0,
+  M_OPT_TYPE_HAS_CHILD,
   parse_dummy,
   NULL,
   NULL,
@@ -79,7 +79,9 @@ static char* dup_printf(const char *fmt,
 #define VAL(x) (*(int*)(x))
 
 static int parse_flag(m_option_t* opt,char *name, const char *param, void* dst, int src) {
-  if (src == M_CONFIG_FILE) {
+  if (src == M_CONFIG_FILE && param == NULL)
+    return M_OPT_MISSING_PARAM;
+  if (param) {
     if (!strcasecmp(param, "yes") ||	/* any other language? */
 	!strcasecmp(param, "ja") ||
 	!strcasecmp(param, "si") ||
@@ -447,7 +449,7 @@ static int parse_choose(m_option_t* opt,
   (void) opt;
   (void) name;
 
-  if (param == NULL || strlen(param) == 0)
+  if (param == NULL || *param == 0)
     return M_OPT_MISSING_PARAM;
 
   if (!dst)
@@ -546,7 +548,6 @@ static int list_del(const char *del, cha
   }
   return ln;
 }
-  
 
 static int parse_list(m_option_t* opt,char *name, const char *param, void* dst, int mode) {
   int pre = 0; /* default to add */
@@ -559,7 +560,7 @@ static int parse_list(m_option_t* opt,ch
     char *n = &name[len];
 
     if(strcasecmp(n,"-clr") == 0) {
-      if(dst)
+      if(dst && mode != M_PREPARSE) // no params need
 	free_list(dst);
       return 0;
     }
@@ -569,10 +570,11 @@ static int parse_list(m_option_t* opt,ch
       pre = -1;
     else if(strcasecmp(n,"-add") != 0)
       return M_OPT_UNKNOW;
-  }
+  } else if (dst && mode != M_PREPARSE && param == NULL)
+    free_list(dst); // empty list before replacing, see m_option_parse_co()
 
   // All other op need a param
-  if (param == NULL || strlen(param) == 0)
+  if (param == NULL || *param == 0)
     return M_OPT_MISSING_PARAM;
 
   if (mode != M_PREPARSE) { // param is existing name to add or del
@@ -597,7 +599,7 @@ static int parse_list(m_option_t* opt,ch
     if (arg)
       *arg++ = 0;
     if (*c == 0) { // empty string
-      free(lst);
+      if (lst) free(lst);
       free(p);
       return M_OPT_INVALID;
     }
@@ -639,7 +641,7 @@ m_option_type_t m_option_type_list = {
   "String list",
   "A list of strings separated by ','\n"
   "Option with name that finish by an * allow to use the following suffix:\n"
-  "\t-add : add the given parameters at the end of list (default)\n"
+  "\t-add : add the given parameters at the end of list\n"
   "\t-pre : add the given parameters at the begining of list\n"
   "\t-del : remove the entries with the given names\n"
   "\t-clr : clear the list\n"
@@ -952,7 +954,7 @@ static int parse_subconf(m_option_t* opt
 m_option_type_t m_option_type_subconfig = {
   "Subconfig",
   "The syntax is -option opt1=foo:flag:opt2=blah",
-  sizeof(int),
+  0,
   M_OPT_TYPE_HAS_CHILD,
   parse_subconf,
   NULL,