[Mesa-dev] [PATCH 3/7] glsl: Add initial functions to implement an on-disk cache

[Carl Worth]

On Wed, Feb 04 2015, Matt Turner wrote:
> Rebase needed. I removed GLSL_SRCDIR a week and a half ago.

Was this the removal of all those subdir-objects warnings? Thank you!

> I don't think it makes a difference to autotools, but I think I'd list
> cache.h here instead of in LIBGLSL_FILES.

Actually, that was sort of intentional. The cache.c file is only conditionally
compiled, but cache.h has the inline stub implementations that we want
to compile unconditionally. So it felt right to put the .h in the
unconditional list.

Of course, maybe that doesn't matter at all. It's the .c files that
include the .h file that are going to result in the inline stubs being
provided. Now that we're on the subject, what does automake even
do with .h files that are listed in these lists? Do they need to be
there for non-srcdir builds to work or something?

If it truly doesn't matter, then I would prefer to see the cache.h next
to the cache.c, yes.

>> +   cache = (struct program_cache *) malloc(sizeof *cache);
>
> Don't cast malloc.

I'll blame krh on this one, (and he can in turn blame it on bad habits
From programming in C++ too much). :-)

>> +   return (void *) data;
>
> I don't think you need this cast either?

Just removing that cast doesn't quite do the trick because there's a
potential signedness difference between char and uint8_t. But the cast
is not the way I want to fix this, so thanks for pointing that out.

(I have gone back and forth on whether this function should return a
uint8_t* or a "void *". There's a later patch in the next series that
still expects a direct assignment from cache_get() to some
data-structure pointer to work without a cast. So I may change cache_get
back to returning a "void *". But even then, the cast above won't be
needed.).

Thanks for your attention to detail.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150204/21157a48/attachment.sig>