On Fri, 2014-10-03 at 13:27 +0300, Francisco Jerez wrote:
> Kenneth Graunke <kenn...@whitecape.org> writes:
>
> > On Thursday, October 02, 2014 11:28:35 AM Ian Romanick wrote:
> >> Since this is Curro's code, I'm CC'ing me.
> >>
> >> On 10/01/2014 03:12 AM, Iago Toral Quiroga wrote:
> >> > in_var calls the ir_variable constructor, which dups the variable name.
> >> > ---
> >> > src/glsl/builtin_functions.cpp | 8 +++++---
> >> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/src/glsl/builtin_functions.cpp
> >> > b/src/glsl/builtin_functions.cpp
> >> > index 5a024cb..7d61fcc 100644
> >> > --- a/src/glsl/builtin_functions.cpp
> >> > +++ b/src/glsl/builtin_functions.cpp
> >> > @@ -4465,9 +4465,11 @@ builtin_builder::_image_prototype(const glsl_type
> >> > *image_type,
> >> > sig->parameters.push_tail(in_var(glsl_type::int_type, "sample"));
> >> >
> >> > /* Data arguments. */
> >> > - for (unsigned i = 0; i < num_arguments; ++i)
> >> > - sig->parameters.push_tail(in_var(data_type,
> >> > - ralloc_asprintf(NULL, "arg%d",
> >> > i)));
> >> > + for (unsigned i = 0; i < num_arguments; ++i) {
> >> > + char *arg_name = ralloc_asprintf(NULL, "arg%d", i);
> >> > + sig->parameters.push_tail(in_var(data_type, arg_name));
> >> > + ralloc_free(arg_name);
> >> > + }
> >>
> >> Using a NULL memory context is generally bad... precisely because it
> >> often leads to memory leaks.
> >>
> >> There are a couple ways to fix this. Since all of the image functions
> >> have a limited number of parameters, we could either:
> >>
> >> - Have a fixed size buffer that we snprintf to.
> >>
> >> - Have a table of all the parameter names.
> >>
> >> - Since this is the function prototype, I don't think we need names for
> >> the parameters at all. Just pass NULL?
> >
> > Does anything even use the names? I don't think anything does...at which
> > point, why not just call them all "arg" and be done with it?
> >
>
> Aren't the names useful for debugging and error reporting? But sure,
> you're right that "arg1" isn't a lot more meaningful than "arg".
Francisco: so I understand that you would be okay with just passing
"arg" for all these parameters as Kenneth suggested? I can send the
patch for this if we agree to do this.
Iago
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
