This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Merge forward-search/reverse-search, use gdb::def_vector, remove limit (Re: [RFA] Fix leak in forward-search)
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>
- Cc: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>, gdb-patches at sourceware dot org
- Date: Sat, 8 Dec 2018 15:12:15 +0000
- Subject: Re: [PATCH] Merge forward-search/reverse-search, use gdb::def_vector, remove limit (Re: [RFA] Fix leak in forward-search)
- References: <20181127233328.5164-1-philippe.waroquiers@skynet.be> <3cf960b8-ff82-0670-fa90-c94d78573bfe@redhat.com> <1543532723.4149.7.camel@skynet.be> <6e8a2a00-bcbd-cdc6-f332-f59988bb8c40@redhat.com> <871s72mh5i.fsf@tromey.com>
On 11/30/2018 08:42 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> -/* FIXME!!! We walk right off the end of buf if we get a long line!!! */
>
> I'm surprised all those exclamation marks didn't help us find this
> sooner. /s
>
> Pedro> I don't think there's much point in reusing the buffer across
> Pedro> command invocations.
>
> Agreed.
>
> Pedro> - msg = (char *) re_comp (regex);
> Pedro> + char *msg = (char *) re_comp (regex);
>
> Pre-existing, but I wonder why this cast is needed. I think "const char
> *msg" and removing the cast ought to be completely fine.
Yeah. I did this, and pushed the patch.
> Or (more work)
> avoid re_comp and use compiled_regex instead.
I looked at this a bit, but decided not to do it, at least not in
this patch, because there are several other re_comp calls in the
tree, which made me think that it'd be better to do a pass
tweaking all the same time. While considering that, it seems like
we end up having to pass a string as 3rd argument to compile_regex's
ctor and many different places, which makes me think that perhaps
that argument should have a default. Similar consideration for
the compile_regex::exec method, and all arguments but the first.
Thanks,
Pedro Alves
>
> The rest seems fine to me. Thank you for doing this.
>
> Tom
>