Re: [Chromium-dev] several bugs in pack/unpacking of textures

[Brian P. <bri...@tu...>]

Greg Humphreys wrote:
> Kevin Dale and I discovered several bugs yesterday in the pack/unpack
> functions for the Compressed texture extensions.  There are 6 such
> functions for 1,2,3D and full/sub textures.
> 
> First of all, packing functions are never responsible for including
> the packet length themselves.  This is done implicitly by crPackAlloc.
>  This causes the unpacker to get confused because the packet looks
> like:
> 
> 4 bytes: length put there by crPackAlloc
> 4 bytes: length (erroneously) put there by the packing function itself
> 4 bytes: extended opcode
> 4 bytes: texture target
> ...
> 
> etc
> 
> but the unpacker tries to interpret the second four bytes as the
> extended opcode and dies.
> 
> Once we fixed this, we realized that the unpack versions of these
> functions didn't exist.  I'm not sure why there are pack functions
> without corresponding unpack functions.  They are very simple to
> implement.

Probably just an oversight.

> I fixed the packing functions, and wrote the unpacker for the one
> function that Kevin was using.  Kevin, would you mind sending the list
> the fixed pack_texture.c and unpack_texture.c so someone could check
> them in (the changes are on his personal machine).
> 
> Also, it seems that we have been lax about surrounding things with
> #ifdefs for extensions.  For example, we tried to undefine
> CR_vertex_buffer_object, but this caused the state tracker to not
> build because it unconditionally uses data structures that are
> conditionally defined.

Yeah, there's probably lots of instances of that problem.  The 
original intention of those preprocessor symbols was to allow omitting 
extension code, but we've seldom actually tested that.

-Brian