<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jun 6, 2018 at 10:25 AM, Lionel Landwerlin <span dir="ltr"><<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 06/06/18 18:17, Jason Ekstrand wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
We add two assertions instead of one because the first assertion that<br>
format != ISL_FORMAT_UNSUPPORTED is more descriptive and checks for a<br>
real but unsupported enumerant while the second ensures that they don't<br>
pass in garbage values.  We also update some other helpers to use<br>
isl_format_get_layout instead of using the table directly so that they<br>
get bounds checking too.<br>
<br>
Cc: <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.<wbr>org</a><br>
---<br>
  src/intel/isl/isl.h | 32 +++++++++++++++++++++---------<wbr>--<br>
  1 file changed, 21 insertions(+), 11 deletions(-)<br>
<br>
diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h<br>
index 00cfe31fc04..6800b1d76a6 100644<br>
--- a/src/intel/isl/isl.h<br>
+++ b/src/intel/isl/isl.h<br>
@@ -389,6 +389,9 @@ enum isl_format {<br>
     ISL_FORMAT_GEN9_CCS_64BPP,<br>
     ISL_FORMAT_GEN9_CCS_128BPP,<br>
  +   /* An upper bound on the supported format enumerations */<br>
+   ISL_NUM_FORMATS,<br>
+<br>
     /* Hardware doesn't understand this out-of-band value */<br>
     ISL_FORMAT_UNSUPPORTED =                             UINT16_MAX,<br>
  };<br>
@@ -1423,6 +1426,8 @@ isl_device_get_sample_counts(s<wbr>truct isl_device *dev);<br>
  static inline const struct isl_format_layout * ATTRIBUTE_CONST<br>
  isl_format_get_layout(enum isl_format fmt)<br>
  {<br>
+   assert(fmt != ISL_FORMAT_UNSUPPORTED);<br>
+   assert(fmt < ISL_NUM_FORMATS);<br>
     return &isl_format_layouts[fmt];<br>
  }<br>
  @@ -1431,7 +1436,7 @@ bool isl_format_is_valid(enum isl_format);<br>
  static inline const char * ATTRIBUTE_CONST<br>
  isl_format_get_name(enum isl_format fmt)<br>
  {<br>
-   return isl_format_layouts[fmt].name;<br>
+   return isl_format_get_layout(fmt)->na<wbr>me;<br>
  }<br>
    bool isl_format_supports_rendering(<wbr>const struct gen_device_info *devinfo,<br>
@@ -1546,7 +1551,7 @@ isl_format_block_is_1x1x1(enum isl_format fmt)<br>
  static inline bool<br>
  isl_format_is_srgb(enum isl_format fmt)<br>
  {<br>
-   return isl_format_layouts[fmt].colors<wbr>pace == ISL_COLORSPACE_SRGB;<br>
+   return isl_format_get_layout(fmt)->co<wbr>lorspace == ISL_COLORSPACE_SRGB;<br>
  }<br>
    enum isl_format isl_format_srgb_to_linear(enum isl_format fmt);<br>
@@ -1556,20 +1561,25 @@ isl_format_is_rgb(enum isl_format fmt)<br>
  {<br>
     if (isl_format_is_yuv(fmt))<br>
        return false;<br>
-   return isl_format_layouts[fmt].channe<wbr>ls.r.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.g.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.b.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.a.bits == 0;<br>
+<br>
+   const struct isl_format_layout *fmtl = isl_format_get_layout(fmt);<br>
+<br>
+   return fmtl->channels.r.bits > 0 &&<br>
+          fmtl->channels.g.bits > 0 &&<br>
+          fmtl->channels.b.bits > 0 &&<br>
+          fmtl->channels.a.bits == 0;<br>
  }<br>
    static inline bool<br>
  isl_format_is_rgbx(enum isl_format fmt)<br>
  {<br>
-   return isl_format_layouts[fmt].channe<wbr>ls.r.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.g.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.b.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.a.bits > 0 &&<br>
-          isl_format_layouts[fmt].channe<wbr>ls.a.type == ISL_VOID;<br>
+   const struct isl_format_layout *fmtl = isl_format_get_layout(fmt);<br>
+<br>
+   return fmtl->channels.r.bits > 0 &&<br>
+          fmtl->channels.g.bits > 0 &&<br>
+          fmtl->channels.b.bits > 0 &&<br>
+          fmtl->channels.a.bits > 0 &&<br>
+          fmtl->channels.a.type == ISL_VOID;<br>
  }<br>
    enum isl_format isl_format_rgb_to_rgba(enum isl_format rgb) ATTRIBUTE_CONST;<br>
</blockquote></div></div>
Do you want to use it in isl_buffer_fill_image_param() too?<br></blockquote><div><br></div><div>Good call.  I've made that change locally.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewed-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a><wbr>><br>
</blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks!<br></div></div>