Re: [Qemu-devel] [PATCH v2 4/4] Replace strotok() by strtok_r()

[Paolo Bonzini]

On 18/09/2015 12:59, address@hidden wrote:
> From: Marc-André Lureau <address@hidden>
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  block/archipelago.c | 10 +++++-----
>  block/sheepdog.c    |  5 +++--
>  qom/cpu.c           | 12 ++++++------
>  target-i386/cpu.c   |  6 +++---
>  target-sparc/cpu.c  | 10 +++++-----
>  vl.c                |  6 +++---
>  6 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/block/archipelago.c b/block/archipelago.c
> index 855655c..1a8fcc0 100644
> --- a/block/archipelago.c
> +++ b/block/archipelago.c
> @@ -354,17 +354,17 @@ static void parse_filename_opts(const char *filename, 
> Error **errp,
>                                  xport *mport, xport *vport)
>  {
>      const char *start;
> -    char *tokens[4], *ds;
> +    char *tokens[4], *ds, *save;
>      int idx;
>      xport lmport = NoPort, lvport = NoPort;
>  
>      strstart(filename, "archipelago:", &start);
>  
>      ds = g_strdup(start);
> -    tokens[0] = strtok(ds, "/");
> -    tokens[1] = strtok(NULL, ":");
> -    tokens[2] = strtok(NULL, ":");
> -    tokens[3] = strtok(NULL, "\0");
> +    tokens[0] = strtok_r(ds, "/", &save);
> +    tokens[1] = strtok_r(NULL, ":", &save);
> +    tokens[2] = strtok_r(NULL, ":", &save);
> +    tokens[3] = strtok_r(NULL, "\0", &save);
>  
>      if (!strlen(tokens[0])) {
>          error_setg(errp, "volume name must be specified first");
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 67ca788..272a4ca 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1614,12 +1614,13 @@ static int parse_redundancy(BDRVSheepdogState *s, 
> const char *opt)
>  {
>      struct SheepdogInode *inode = &s->inode;
>      const char *n1, *n2;
> +    char *save;
>      long copy, parity;
>      char p[10];
>  
>      pstrcpy(p, sizeof(p), opt);
> -    n1 = strtok(p, ":");
> -    n2 = strtok(NULL, ":");
> +    n1 = strtok_r(p, ":", &save);
> +    n2 = strtok_r(NULL, ":", &save);
>  
>      if (!n1) {
>          return -EINVAL;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index fb80d13a..9511f10 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -42,14 +42,14 @@ bool cpu_exists(int64_t id)
>  
>  CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
>  {
> -    char *str, *name, *featurestr;
> +    char *str, *name, *featurestr, *save;
>      CPUState *cpu;
>      ObjectClass *oc;
>      CPUClass *cc;
>      Error *err = NULL;
>  
>      str = g_strdup(cpu_model);
> -    name = strtok(str, ",");
> +    name = strtok_r(str, ",", &save);
>  
>      oc = cpu_class_by_name(typename, name);
>      if (oc == NULL) {
> @@ -60,7 +60,7 @@ CPUState *cpu_generic_init(const char *typename, const char 
> *cpu_model)
>      cpu = CPU(object_new(object_class_get_name(oc)));
>      cc = CPU_GET_CLASS(cpu);
>  
> -    featurestr = strtok(NULL, ",");
> +    featurestr = strtok_r(NULL, ",", &save);
>      cc->parse_features(cpu, featurestr, &err);
>      g_free(str);
>      if (err != NULL) {
> @@ -276,10 +276,10 @@ static void cpu_common_parse_features(CPUState *cpu, 
> char *features,
>                                        Error **errp)
>  {
>      char *featurestr; /* Single "key=value" string being parsed */
> -    char *val;
> +    char *val, *save;
>      Error *err = NULL;
>  
> -    featurestr = features ? strtok(features, ",") : NULL;
> +    featurestr = features ? strtok_r(features, ",", &save) : NULL;
>  
>      while (featurestr) {
>          val = strchr(featurestr, '=');
> @@ -296,7 +296,7 @@ static void cpu_common_parse_features(CPUState *cpu, char 
> *features,
>                         featurestr);
>              return;
>          }
> -        featurestr = strtok(NULL, ",");
> +        featurestr = strtok_r(NULL, ",", &save);
>      }
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index d2b6bc5..2487641 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1851,7 +1851,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
> *features,
>                                       Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> -    char *featurestr; /* Single 'key=value" string being parsed */
> +    char *save, *featurestr; /* Single 'key=value" string being parsed */
>      FeatureWord w;
>      /* Features to be added */
>      FeatureWordArray plus_features = { 0 };
> @@ -1861,7 +1861,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
> *features,
>      CPUX86State *env = &cpu->env;
>      Error *local_err = NULL;
>  
> -    featurestr = features ? strtok(features, ",") : NULL;
> +    featurestr = features ? strtok_r(features, ",", &save) : NULL;
>  
>      while (featurestr) {
>          char *val;
> @@ -1930,7 +1930,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
> *features,
>              error_propagate(errp, local_err);
>              return;
>          }
> -        featurestr = strtok(NULL, ",");
> +        featurestr = strtok_r(NULL, ",", &save);
>      }
>  
>      if (cpu->host_features) {
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index 9528e3a..6269091 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -95,7 +95,7 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
> *cpu_model)
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUSPARCState *env = &cpu->env;
>      char *s = g_strdup(cpu_model);
> -    char *featurestr, *name = strtok(s, ",");
> +    char *featurestr, *name = strtok_r(s, ",", &save);
>      sparc_def_t def1, *def = &def1;
>      Error *err = NULL;
>  
> @@ -107,7 +107,7 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
> *cpu_model)
>      env->def = g_new0(sparc_def_t, 1);
>      memcpy(env->def, def, sizeof(*def));
>  
> -    featurestr = strtok(NULL, ",");
> +    featurestr = strtok_r(NULL, ",", &save);
>      cc->parse_features(CPU(cpu), featurestr, &err);
>      g_free(s);
>      if (err) {
> @@ -560,13 +560,13 @@ static void sparc_cpu_parse_features(CPUState *cs, char 
> *features,
>  {
>      SPARCCPU *cpu = SPARC_CPU(cs);
>      sparc_def_t *cpu_def = cpu->env.def;
> -    char *featurestr;
> +    char *featurestr, *save;
>      uint32_t plus_features = 0;
>      uint32_t minus_features = 0;
>      uint64_t iu_version;
>      uint32_t fpu_version, mmu_version, nwindows;
>  
> -    featurestr = features ? strtok(features, ",") : NULL;
> +    featurestr = features ? strtok_r(features, ",", &save) : NULL;
>      while (featurestr) {
>          char *val;
>  
> @@ -634,7 +634,7 @@ static void sparc_cpu_parse_features(CPUState *cs, char 
> *features,
>                               "(+feature|-feature|feature=xyz)", featurestr);
>              return;
>          }
> -        featurestr = strtok(NULL, ",");
> +        featurestr = strtok_r(NULL, ",", &save);
>      }
>      cpu_def->features |= plus_features;
>      cpu_def->features &= ~minus_features;
> diff --git a/vl.c b/vl.c
> index 3c6480d..bab2696 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1323,16 +1323,16 @@ static int add_semihosting_arg(void *opaque,
>  /* Use strings passed via -kernel/-append to initialize semihosting.argv[] */
>  static inline void semihosting_arg_fallback(const char *file, const char 
> *cmd)
>  {
> -    char *cmd_token;
> +    char *cmd_token, *save;
>  
>      /* argv[0] */
>      add_semihosting_arg(&semihosting, "arg", file, NULL);
>  
>      /* split -append and initialize argv[1..n] */
> -    cmd_token = strtok(g_strdup(cmd), " ");
> +    cmd_token = strtok_r(g_strdup(cmd), " ", &save);
>      while (cmd_token) {
>          add_semihosting_arg(&semihosting, "arg", cmd_token, NULL);
> -        cmd_token = strtok(NULL, " ");
> +        cmd_token = strtok_r(NULL, " ", &save);
>      }
>  }
>  
> 

Unfortunately mingw doesn't have strtok_r (I think).  I have queued v1
of your patches, if you want to write more tests please send them on top.

Paolo