|
From: Robert W. <rj...@du...> - 2006-02-04 21:49:15
|
Hi all,
Valgrind still has some nested functions in m_debuginfo/symtypes.c
(newvar and genstring). What's the policy with respect to nested
functions? Would you all be OK if I recoded this a little to remove the
nesting? I only noticed them because my compile threw up this warning:
m_debuginfo/symtypes.c: In function 'vgPlain_describe_addr':
m_debuginfo/symtypes.c:765: warning: no previous prototype for 'newvar'
m_debuginfo/symtypes.c:986: warning: no previous prototype for 'genstri=
ng'
This isn't a problem for me, as such, but I have a vague recollection of
people on the list reporting that this caused compile problems on older
versions of gcc.
Regards,
Robert.
--=20
Robert Walsh
Amalgamated Durables, Inc. - "We don't make the things you buy."
Email: rj...@du...
|
|
From: Julian S. <js...@ac...> - 2006-02-05 00:28:09
|
> Valgrind still has some nested functions in m_debuginfo/symtypes.c > (newvar and genstring). What's the policy with respect to nested > functions? Would you all be OK if I recoded this a little to remove the > nesting? I'd love to be rid of them. That'd be great, yes please. As far as I know they are the only thing standing in the way of being able to build V with icc, although that's based on experimentation from long ago. I don't know what level of gnuisms pathcc can cope with, but if it was also pathcc-buildable that would be good. The real issue is that we've had various regressions in debuginfo reading for non-gcc compilers because the test suite is only routinely built with gcc. So - technically - we could avoid such regressions by building the testsuite with non-gcc, but in general the wider the set of compilers that can build the whole thing successfully, the better, imo. Oh .. and the warnings bug me too. J |
|
From: Nicholas N. <nj...@cs...> - 2006-02-05 01:04:54
|
On Sun, 5 Feb 2006, Julian Seward wrote: >> Valgrind still has some nested functions in m_debuginfo/symtypes.c >> (newvar and genstring). What's the policy with respect to nested >> functions? Would you all be OK if I recoded this a little to remove the >> nesting? > > I'd love to be rid of them. That'd be great, yes please. I agree. I tried once, but those two are tricky suckers and I didn't want to break anything so I didn't. They're in the the debug sym code that only Helgrind uses, and so it may have bitrotted somewhat. N |
|
From: Robert W. <rj...@du...> - 2006-02-06 05:48:29
Attachments:
nested.diff
|
> I agree. I tried once, but those two are tricky suckers and I didn't w= ant > to break anything so I didn't. They're in the the debug sym code that > only Helgrind uses, and so it may have bitrotted somewhat. Before I left for my conference, I did a hack fix that moved both nested functions out and passed in pointers to any variables they modified. It compiled and no extra tests failed, but as I said, it was hacky. Patch i= s attached. If you're all OK with it, I'll commit it. Regards, Robert. --=20 Robert Walsh Amalgamated Durables, Inc. "We bring dead things to life" |
|
From: Julian S. <js...@ac...> - 2006-02-10 00:27:46
|
Robert, Apologies for delayed reply. > Before I left for my conference, I did a hack fix that moved both nested > functions out and passed in pointers to any variables they modified. It > compiled and no extra tests failed, but as I said, it was hacky. Patch is > attached. If you're all OK with it, I'll commit it. It looks ok to me. My only reservation is - have you been able to run tests to verify that said functionality still works post change? J |
|
From: Robert W. <rj...@du...> - 2006-02-10 02:02:59
|
On Fri, 2006-02-10 at 00:27 +0000, Julian Seward wrote: > Robert, > > Apologies for delayed reply. > > > Before I left for my conference, I did a hack fix that moved both nested > > functions out and passed in pointers to any variables they modified. It > > compiled and no extra tests failed, but as I said, it was hacky. Patch is > > attached. If you're all OK with it, I'll commit it. > > It looks ok to me. My only reservation is - have you been able to > run tests to verify that said functionality still works post change? Well, all the current test suite still runs with no changes. Now whether that means the functionality is still being tested, I honestly can't say. |
|
From: Robert W. <rj...@du...> - 2006-02-10 03:17:45
|
On Fri, 2006-02-10 at 14:15 +1100, Nicholas Nethercote wrote: > On Thu, 9 Feb 2006, Robert Walsh wrote: >=20 > >> It looks ok to me. My only reservation is - have you been able to > >> run tests to verify that said functionality still works post change? > > > > Well, all the current test suite still runs with no changes. Now > > whether that means the functionality is still being tested, I honestly > > can't say. >=20 > It's not -- that code is for the stabs symbol stuff that is only used by=20 > Helgrind. Well, that leaves me in a bit of a pickle, then. Since Helgrind is probably bitrotted by now, I'd say I should just check it in and we can fix it if it breaks anything later. That sound OK? --=20 Robert Walsh Amalgamated Durables, Inc. - "We don't make the things you buy." Email: rj...@du... |
|
From: Nicholas N. <nj...@cs...> - 2006-02-10 03:24:11
|
On Thu, 9 Feb 2006, Robert Walsh wrote: > Well, that leaves me in a bit of a pickle, then. Since Helgrind is > probably bitrotted by now, I'd say I should just check it in and we can > fix it if it breaks anything later. That sound OK? Ok by me. |
|
From: Julian S. <js...@ac...> - 2006-02-10 11:52:09
|
On Friday 10 February 2006 03:23, Nicholas Nethercote wrote: > On Thu, 9 Feb 2006, Robert Walsh wrote: > > Well, that leaves me in a bit of a pickle, then. Since Helgrind is > > probably bitrotted by now, I'd say I should just check it in and we can > > fix it if it breaks anything later. That sound OK? > > Ok by me. Me too (retrospectively). It's good to be rid of those warnings. Thanks for doing that. J |