On 10/28/2011 10:42 AM, Ian Romanick wrote: > From: Ian Romanick <ian.d.roman...@intel.com> > > Also prepend _mesa_uniform_ to the names. > > Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> ... > -static void > -merge_location_offset(GLint *location, GLint offset) > -{ > - *location = (*location << 16) | offset; > -} ... > +/** > + * Combine the uniform's base location and the offset > + */ > +static inline GLint > +_mesa_uniform_merge_location_offset(unsigned base_location, unsigned offset) > +{ > + return (base_location << 16) | offset; > +} > +
You've clearly done more than what your commit message says here...you changed this function's calling convention from foo(&x,y) to x=foo(x,y) and rewrote the callers appropriately. While I'm not opposed to that, it's worth mentioning. > -/** > - * Separate the uniform location and parameter offset. See above. > - */ > -static void > -split_location_offset(GLint *location, GLint *offset) > -{ > - *offset = *location & 0xffff; > - *location = *location >> 16; > -} ... > +/** > + * Separate the uniform base location and parameter offset > + */ > +static inline void > +_mesa_uniform_split_location_offset(GLint location, unsigned *base_location, > + unsigned *offset) > +{ > + *offset = location & 0xffff; > + *base_location = location >> 16; > +} This one is even stranger. You added a third parameter and rewrote the existing callers to do: _mesa_uniform_split_location_offset(location, &location, offset). This seems rather weird and redundant at first glance. Though, I think I see your rationale: location is an input parameter containing base+offset (together), while the next two are output parameters for base and offset (separately). The former function muddled things by forcing you to override the combined input, which is a pain for callers that want to keep all three. So I suppose I'm not opposed to this either, but please do mention/justify the change in the commit message. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev