See below for modified patch, indentation and newline for curly braces
style applied, and commented out chunk removed. Apologies, indentation
and newline for scope are not they way I normally write things, habits
got the better of me, and I forgot to remove the commented out chunk
before submission.
Adding the scope braces and writing the for loop in ordinary C instead
of relying on a macro are changes made for the sake of
maintainability. This feature is rarely touched (as evidenced by it
having the bugs I'm fixing for so long) and so it is more likely any
patches submitted to it will be submitted by people who a) are not
maintainers of GCC, and so are not familiar with the various macros
etc "normally" used and b) don't have the time / patience to
familiarize themselves with the code base before fixing just the bit
they need to work for whatever they're doing. For people like that
(people like me), having the code be written in as obvious a way as
possible and clearly marking scope of branches makes making fixes
easier (adding a quick printf for debugging is painless, etc), which
is important for low usage features.
Index: gimplify.c
===================================================================
--- gimplify.c 2019-06-12 19:07:26.872077000 +0100
+++ gimplify.c 2019-07-04 08:53:08.768420000 +0100
@@ -13987,11 +13987,16 @@ flag_instrument_functions_exclude_p (tre
{
const char *name;
- int i;
+ unsigned int i;
char *s;
- name = lang_hooks.decl_printable_name (fndecl, 0);
- FOR_EACH_VEC_ELT (*v, i, s)
- if (strstr (name, s) != NULL)
- return true;
+ name = lang_hooks.decl_printable_name (fndecl, 1);
+ for(i = 0; i < v->length(); i++)
+ {
+ s = (*v)[i];
+ if(strstr(name, s) != NULL)
+ {
+ return(true);
+ }
+ }
}
Index: opts.c
===================================================================
--- opts.c 2019-06-12 19:10:04.354612000 +0100
+++ opts.c 2019-07-04 08:55:25.287523000 +0100
@@ -263,7 +263,9 @@ add_comma_separated_to_vector (void **pv
*w++ = *r++;
}
+ *w = '\0';
if (*token_start != '\0')
+ {
v->safe_push (token_start);
-
+ }
*pvec = v;
}
On Thu, Jul 4, 2019 at 12:31 AM Jeff Law <l...@redhat.com> wrote:
>
> On 6/12/19 12:25 PM, Oliver Browne wrote:
> > Patch fixes following PRs:
> > c++/90816 - -finstrument-functions-exclude-function-list improperly
> > handles namespace/class definitions
> > c++/90809 - -finstrument-functions-exclude-function-list mishandles
> > comma escaping
> >
> > Fixes as follows:
> > At flag_instrument_functions_exclude_p [gimplify.c]
> > Using lang_hooks.decl_printable_name (fndecl, 1) to get namespace /
> > class information as part of printable name to allow for
> > inclusion of namespace / class specification when passing symbols to
> > -finstrument-functions-exclude-function-list. Was
> > previously lang_hooks.decl_printable_name (fndecl, 0).
> >
> > At add_comma_separated_to_vector [opts.c]
> > Added writing of a null character to w after primary loop finishes, to
> > account for offset between r and w when r reaches end of
> > passed string.
> >
> > from Oliver Browne <oliverbrowne...@gmail.com>
> > PR c++/90816
> > PR c++/90809
> > * gimplify.c (flag_instrument_functions_exclude_p): include namespace
> > information as part of decl name
> > * opts.c (add_comma_separated_to_vector): add null character to correct
> > position in last token added to token vector
> > Index: gimplify.c
> > ===================================================================
> > --- gimplify.c 2019-06-12 19:07:26.872077000 +0100
> > +++ gimplify.c 2019-06-12 18:55:10.609255000 +0100
> > @@ -13987,11 +13987,17 @@ flag_instrument_functions_exclude_p (tre
> > {
> > const char *name;
> > - int i;
> > + unsigned int i;
> > char *s;
> >
> > - name = lang_hooks.decl_printable_name (fndecl, 0);
> > - FOR_EACH_VEC_ELT (*v, i, s)
> > + name = lang_hooks.decl_printable_name (fndecl, 1);
> > + for(i = 0; i < v->length(); i++){
> > + s = (*v)[i];
> > + if(strstr(name, s) != NULL){
> > + return(true);
> > + }
> > + }
> > +/* FOR_EACH_VEC_ELT (*v, i, s)
> > if (strstr (name, s) != NULL)
> > - return true;
> > + return true;*/
> > }
> So why did you drop the FOR_EACH_VEC_ELT and open-code the loop? I
> don't see that as being a necessary change. Leaving the
> FOR_EACH_VEC_ELT in place would also avoid the mis-formatting you've
> introduced as well as removing clutter of a commented-out hunk of code.
>
> >
> > @@ -14278,3 +14284,3 @@ gimplify_hasher::equal (const elt_t *p1,
> >
> > return true;
> > -}
> > \ No newline at end of file
> > +}
> > Index: opts.c
> > ===================================================================
> > --- opts.c 2019-06-12 19:10:04.354612000 +0100
> > +++ opts.c 2019-06-12 18:53:43.675852000 +0100
> > @@ -263,7 +263,8 @@ add_comma_separated_to_vector (void **pv
> > *w++ = *r++;
> > }
> > - if (*token_start != '\0')
> > + *w = '\0';
> > + if (*token_start != '\0'){
> > v->safe_push (token_start);
> > -
> > + }
> > *pvec = v;
> So why introduce the unnecessary { } scope? And why do it in a way that
> is different than 99% of the other code in GCC (where the { and } will
> be on individual lines with 2 spaces of indention?
>
> Jeff
