On 09/13/2012 02:44 PM, Kristian Høgsberg wrote:
On Tue, Sep 11, 2012 at 1:13 PM, Brian Paul<bri...@vmware.com> wrote:
On 09/11/2012 10:36 AM, Kristian Høgsberg wrote:
On Tue, Sep 11, 2012 at 11:47 AM, Brian Paul<bri...@vmware.com> wrote:
On 09/10/2012 12:41 AM, Imre Deak wrote:
When traversing the hash table looking up an enum that is invalid we
eventually reach the first element in the descriptor array. By looking
at the type of that element, which is always TYPE_API_MASK, we know that
we can stop the search and return error. Since this element is always
the first it's enough to check for its index being 0 without looking at
its type.
Later in this patchset, when we generate the hash tables during build
time, this will allow us to remove the TYPE_API_MASK and related flags
completly.
Signed-off-by: Imre Deak<imre.d...@intel.com>
---
src/mesa/main/get.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 63fe296..48c6911 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -2012,16 +2012,18 @@ find_value(const char *func, GLenum pname, void
**p, union value *v)
mask = Elements(table(api)) - 1;
hash = (pname * prime_factor);
while (1) {
- d =&values[table(api)[hash& mask]];
+ int idx = table(api)[hash& mask];
/* If the enum isn't valid, the hash walk ends with index 0,
- * which is the API mask entry at the beginning of values[]. */
- if (unlikely(d->type == TYPE_API_MASK)) {
+ * pointing to the first entry of values[] which doesn't hold
+ * any valid enum. */
+ if (unlikely(!idx)) {
Minor nit, but I think "idx != 0" would be nicer here.
_mesa_error(ctx, GL_INVALID_ENUM, "%s(pname=%s)", func,
_mesa_lookup_enum_by_nr(pname));
return&error_value;
}
+ d =&values[idx];
if (likely(d->pname == pname))
break;
To be honest, I'm not quite sure I understand how this code works and how
we
wind up at entry[0] for an invalid enum.
We use a hash table to map enum values to the index into the
corresponding value_desc table. The first entry in value_desc table
is not a valid enum description, so we use index 0 as a terminator for
the hash chains. So if we end up looking at value_desc[0], it means
that we reached the end of a hash chain and didn't file the value we
were looking for.
And by always stepping by the prime step we'll eventually wind up at index
0, right? I think that's the subtle part that could use a comment.
No, nothing that subtle. The hash function will give an initial index
into 'table', which just has an index into 'values'. The values
entry is the description of how to find the value. If that matches
the enum, we were trying to look up we're done. Otherwise we add a
prime step to the index into 'table' and look up a new index into
'values' there. We keep going like this until the index we look up in
'table' is 0, which is not a valid entry into 'values'. There's no
tricky module arithmetic going on there, we just reached an entry in
'table' that no enum hashed to, which means that the enum we're trying
to look up is not in the table.
Ah, OK. I think i was confusing probing the hash table vs. the values
table. I still think a comment would be helpful. :)
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
