Message ID | 8762cvmpd0.fsf_-_@rho.meyering.net |
---|---|
State | New |
Headers | show |
On Thu, Apr 19, 2012 at 10:35 PM, Jim Meyering <jim@meyering.net> wrote: > Richard Guenther wrote: >> Sure, my point was that the >> >> if (strlen (m->name) >= sizeof buf) >> { >> error ("%s:%d:mode name \"%s\" is too long", >> m->file, m->line, m->name); >> continue; >> } >> >> check does not account for the (conditional) prepending of 'C' and the >> snprintf would silently discard the last character of the name. > > Here's a patch for that. (passes make bootstrap) > Incidentally it removes the name-length limitation of 7 and it hoists > the xmalloc, which induces the single added free-before-continue. > There was some clean-up along the way: "q" was easy to eliminate, and > I pulled the two identical snprintf calls out of the "if" block and > replaced them with the buf[0] assignment and memcpy. Ok. Thanks, Richard. > > 2012-04-19 Jim Meyering <meyering@redhat.com> > > * genmodes.c (make_complex_modes): Don't truncate a mode name of > length 7 or more when prepending a "C". Suggested by Richard Guenther. > > > gcc/genmodes.c | 32 +++++++++++++------------------- > 1 file changed, 13 insertions(+), 19 deletions(-) > > diff --git a/gcc/genmodes.c b/gcc/genmodes.c > index 6bbeb6f..84517b9 100644 > --- a/gcc/genmodes.c > +++ b/gcc/genmodes.c > @@ -427,7 +427,6 @@ make_complex_modes (enum mode_class cl, > { > struct mode_data *m; > struct mode_data *c; > - char buf[8]; > enum mode_class cclass = complex_class (cl); > > if (cclass == MODE_RANDOM) > @@ -435,6 +434,7 @@ make_complex_modes (enum mode_class cl, > > for (m = modes[cl]; m; m = m->next) > { > + char *p, *buf; > size_t m_len; > > /* Skip BImode. FIXME: BImode probably shouldn't be MODE_INT. */ > @@ -442,40 +442,34 @@ make_complex_modes (enum mode_class cl, > continue; > > m_len = strlen (m->name); > - if (m_len >= sizeof buf) > - { > - error ("%s:%d:mode name \"%s\" is too long", > - m->file, m->line, m->name); > - continue; > - } > + /* The leading "1 +" is in case we prepend a "C" below. */ > + buf = (char *) xmalloc (1 + m_len + 1); > > /* Float complex modes are named SCmode, etc. > Int complex modes are named CSImode, etc. > This inconsistency should be eliminated. */ > + p = 0; > if (cl == MODE_FLOAT) > { > - char *p, *q = 0; > - /* We verified above that m->name+NUL fits in buf. */ > memcpy (buf, m->name, m_len + 1); > p = strchr (buf, 'F'); > - if (p == 0) > - q = strchr (buf, 'D'); > - if (p == 0 && q == 0) > + if (p == 0 && strchr (buf, 'D') == 0) > { > error ("%s:%d: float mode \"%s\" has no 'F' or 'D'", > m->file, m->line, m->name); > + free (buf); > continue; > } > - > - if (p != 0) > - *p = 'C'; > - else > - snprintf (buf, sizeof buf, "C%s", m->name); > } > + if (p != 0) > + *p = 'C'; > else > - snprintf (buf, sizeof buf, "C%s", m->name); > + { > + buf[0] = 'C'; > + memcpy (buf + 1, m->name, m_len + 1); > + } > > - c = new_mode (cclass, xstrdup (buf), file, line); > + c = new_mode (cclass, buf, file, line); > c->component = m; > } > } > -- > 1.7.10.208.gb4267
Richard Guenther wrote: > On Thu, Apr 19, 2012 at 10:35 PM, Jim Meyering <jim@meyering.net> wrote: >> Richard Guenther wrote: >>> Sure, my point was that the >>> >>> if (strlen (m->name) >= sizeof buf) >>> { >>> error ("%s:%d:mode name \"%s\" is too long", >>> m->file, m->line, m->name); >>> continue; >>> } >>> >>> check does not account for the (conditional) prepending of 'C' and the >>> snprintf would silently discard the last character of the name. >> >> Here's a patch for that. (passes make bootstrap) >> Incidentally it removes the name-length limitation of 7 and it hoists >> the xmalloc, which induces the single added free-before-continue. >> There was some clean-up along the way: "q" was easy to eliminate, and >> I pulled the two identical snprintf calls out of the "if" block and >> replaced them with the buf[0] assignment and memcpy. > > Ok. Thanks for the review. Committed.
diff --git a/gcc/genmodes.c b/gcc/genmodes.c index 6bbeb6f..84517b9 100644 --- a/gcc/genmodes.c +++ b/gcc/genmodes.c @@ -427,7 +427,6 @@ make_complex_modes (enum mode_class cl, { struct mode_data *m; struct mode_data *c; - char buf[8]; enum mode_class cclass = complex_class (cl); if (cclass == MODE_RANDOM) @@ -435,6 +434,7 @@ make_complex_modes (enum mode_class cl, for (m = modes[cl]; m; m = m->next) { + char *p, *buf; size_t m_len; /* Skip BImode. FIXME: BImode probably shouldn't be MODE_INT. */ @@ -442,40 +442,34 @@ make_complex_modes (enum mode_class cl, continue; m_len = strlen (m->name); - if (m_len >= sizeof buf) - { - error ("%s:%d:mode name \"%s\" is too long", - m->file, m->line, m->name); - continue; - } + /* The leading "1 +" is in case we prepend a "C" below. */ + buf = (char *) xmalloc (1 + m_len + 1); /* Float complex modes are named SCmode, etc. Int complex modes are named CSImode, etc. This inconsistency should be eliminated. */ + p = 0; if (cl == MODE_FLOAT) { - char *p, *q = 0; - /* We verified above that m->name+NUL fits in buf. */ memcpy (buf, m->name, m_len + 1); p = strchr (buf, 'F'); - if (p == 0) - q = strchr (buf, 'D'); - if (p == 0 && q == 0) + if (p == 0 && strchr (buf, 'D') == 0) { error ("%s:%d: float mode \"%s\" has no 'F' or 'D'", m->file, m->line, m->name); + free (buf); continue; } - - if (p != 0) - *p = 'C'; - else - snprintf (buf, sizeof buf, "C%s", m->name); } + if (p != 0) + *p = 'C'; else - snprintf (buf, sizeof buf, "C%s", m->name); + { + buf[0] = 'C'; + memcpy (buf + 1, m->name, m_len + 1); + } - c = new_mode (cclass, xstrdup (buf), file, line); + c = new_mode (cclass, buf, file, line); c->component = m; } }