This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Fix a (one shot small) leak in language.c


On Wed, 2018-12-05 at 11:06 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> It is unclear why a xstrdup-ed value is assigned to 'language'
> Philippe> at initialization time, while a static "auto" string is assigned
> Philippe> as part of the set_language_command.
> 
> git annotate shows it is ancient, so it's hard to say -- maybe things
> were different back then or maybe it's just an ancient bug.
I think that this was needed a long time ago: there was no enum
to store the language/range/case_sensitive value. Instead, the value was
stored as a dynamically allocated string.
The xstrdup became useless when the enums were introduced.

> 
> Philippe> 	* language.c (_initialize_language): Fix leak by assigning
> Philippe> 	a static string to language.  Same for range and case_sensitive,
> Philippe> 	even if no leak is detected for these variables.
> 
> I think technically the value should come from the array passed to
> add_setshow_enum_cmd, but in practice that only matters if the values
> are compared using ==, which is not the case here; and in any case the
> current way (comparing using strcmp) seems more clear and thus
> preferable.
> 
> So, this is ok.  Thanks.
Thanks for the review, pushed.

Philippe


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]