[Cogl] [PATCH 1/2] gpu-info: Detect more info including architecture

[Neil Roberts]

Robert Bragg <robert at sixbynine.org> writes:

> +static CoglBool
> +check_sgx_architecture (const CoglGpuInfoStrings *strings)
> +{
> +  if (strncmp (strings->renderer_string, "PowerVR ", 9) != 0)
> +    return FALSE;
> +
> +  /* In some cases you'll see "PowerVR SGX535" and in others it may be
> +   * "PowerVR SGX 535"
> +   */
> +  if (strncmp (&strings->renderer_string[8], "SGX", 3) != 0)
> +    return FALSE;

Why does it do this comparison in two parts? Isn't this equivalent to
just strncmp (strings->renderer_string, "PowerVR SGX", 12)?

> +static CoglBool
> +check_mesa_vendor (const CoglGpuInfoStrings *strings)
> +{
> +  if (strcmp (strings->vendor_string, "Tungsten Graphics, Inc") == 0)
> +    return TRUE;
> +  else if (strcmp (strings->vendor_string, "VMware, Inc.") == 0)
> +    return TRUE;
> +
> +  return TRUE;
> +}

This function always returns TRUE!

>      /* Must be last */
>      {
> +      COGL_GPU_INFO_VENDOR_MESA,
> +      "Mesa",
> +      check_mesa_vendor,
> +      mesa_architectures
> +    },
> +    {
>        COGL_GPU_INFO_VENDOR_UNKNOWN,
>        "Unknown",
> -      check_unknown_vendor
> +      check_true
>      }
>    };

It looks like the 'must be last' comment ends up in the wrong place
after this patch.

The 'unknown' vendor doesn't have any architectures so the pointer
remains NULL and it will crash if its actually hit.

Otherwise looks good to me.

Reviewed by: Neil Roberts <neil at linux.intel.com>

- Neil