[Pixman] [PATCH 05/12] MIPS: MIPS32r2: Added support for run-time detection

[Søren Sandmann]

Nemanja Lukic <nemanja.lukic at rt-rk.com> writes:

> +#ifdef USE_MIPS_DSPR2
> +static const char *mips_dspr2_cores[] = {"MIPS 74K", NULL};
> +#endif
> +
> +#ifdef USE_MIPS32R2
> +static const char *mips32r2_cores[] = {"MIPS 1004K", "MIPS 74K", "MIPS 34K",
> +                                       "MIPS 24K", "MIPS 4Kc", "MIPS 4Km",
> +                                       "MIPS 4Kp", "MIPS 4KEc", "MIPS 4KEm",
> +                                       "MIPS 4KEp", "MIPS 4KSc", "MIPS 4KSd",
> +                                        NULL};
> +#endif
> +
> +#ifdef USE_LOONGSON_MMI
> +static const char *mips_loongson_cores[] = {"Loongson", NULL};
> +#endif

A coding style nitpick: Please format these arrays like this:

    static const char *mips32r2_cores[] = 
    {
        "blah", "blah blah", "asdfasd",
        "blah", "asdfa",
    };
    
>  #if defined(USE_MIPS_DSPR2) || defined(USE_MIPS32R2) || \
>      defined(USE_LOONGSON_MMI)
>  
>  static pixman_bool_t
> -have_feature (const char *search_string)
> +have_feature (const char **cores)
>  {
>  #if defined (__linux__) /* linux ELF */
>      /* Simple detection of MIPS features at runtime for Linux.
> @@ -48,13 +63,18 @@ have_feature (const char *search_string)
>      if ((f = fopen (file_name, "r")) == NULL)
>          return FALSE;
>  
> -    while (fgets (cpuinfo_line, sizeof (cpuinfo_line), f) != NULL)
> +    while (*cores)
>      {
> -        if (strstr (cpuinfo_line, search_string) != NULL)
> +        while (fgets (cpuinfo_line, sizeof (cpuinfo_line), f) != NULL)
>          {
> -            fclose (f);
> -            return TRUE;
> +            if (strstr (cpuinfo_line, *cores) != NULL)
> +            {
> +                fclose (f);
> +                return TRUE;
> +            }
>          }
> +        rewind (f);
> +        cores++;
>      }

It probably doesn't matter that much in practice, but it bothers me that
this code does:

        for each core:
            read /proc file

which I think is both slower and less elegant (due to the rewind() call)
than:

        for each line in /proc file:
            for each core
                ...

Maybe I'm missing something though.


Søren