On May 22, Jay McCarthy wrote:
> On Sat, May 22, 2010 at 4:47 AM, Eli Barzilay <e...@barzilay.org> wrote:
> > On May 21, j...@racket-lang.org wrote:
> >> src/foreign/gcc/libffi/src/closures.c
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> --- OLD/src/foreign/gcc/libffi/src/closures.c
> >> +++ NEW/src/foreign/gcc/libffi/src/closures.c
> >> @@ -379,8 +379,12 @@ dlmmap_locked (void *start, size_t length, int prot,
> >> int flags, off_t offset)
> >> close (execfd);
> >> goto retry_open;
> >> }
> >> - ftruncate (execfd, offset);
> >> - return MFAIL;
> >> + if (ftruncate (execfd, offset)) {
> >> + // FIXME: Should fail "even worse" because the truncate
> >> failed
> >> + return MFAIL;
> >> + } else {
> >> + return MFAIL;
> >> + }
> >
> > Now that I looked at this, it's not clear to me what you're trying to
> > get out of this -- you didn't change anything, and since this is C,
> > there's no real way to "fail worse" besides segfaulting unless you
> > rewrite more code an introduce new special return values etc.
>
> ftruncate is flagged as "warn if return is ignored" on linux, so if
> the return is not used there's an annoying error. I don't know what
> this code is doing exactly, but presumably it should do "something
> different" if the file truncate fails. By inserting the if, the return
> is handled and the error is gone.
Can you do it in a more obvious way, like
/* blah blah */
return ftruncate(...) ? MFAIL : MFAIL;
? -- this would also make a smaller change from libffi.
> >> src/gracket/gracket.cxx
> >> ~~~~~~~~~~~~~~~~~~~~~~~
> >> --- OLD/src/gracket/gracket.cxx
> >> +++ NEW/src/gracket/gracket.cxx
> >> @@ -19,6 +19,7 @@
> >> /* wx_motif, for wxTimer: */
> >> #ifdef __GNUG__
> >> # pragma implementation "wx_timer.h"
> >> +#pragma GCC diagnostic ignored "-Wwrite-strings"
> >> #endif
> >
> > I get tons of "warning: ignoring #pragma GCC diagnostic" in the build.
> > (I haven't looked at which platforms, but it's not a single one.)
>
> That's strange because that's why I put them in __GNUG__. Perhaps it
> is only supported by a newer version of GCC?
For more details (like which platforms this failed on) look at the
build log.
> >> src/gracket/wxs/list.xci
> >> ~~~~~~~~~~~~~~~~~~~~~~~~
> >> --- OLD/src/gracket/wxs/list.xci
> >> +++ NEW/src/gracket/wxs/list.xci
> >> [...]
> >> -static Scheme_Object *l_MAKE_LIST(l_TYPE l_POINT *f, l_INTTYPE c)
> >> +MAYBE_UNUSED static Scheme_Object *l_MAKE_LIST(l_TYPE l_POINT *f,
> >> l_INTTYPE c)
> >
> > This (and many others) looks suspicious.
>
> Even though Matthew said it's ok. Here's some background: Sometimes
> the xc creates lists and vectors that they only traverse (because
> they came from other files) but they define the same functions many
> times, so gcc complains that you're not using some of the functions
> you define.
The thing that looked suspicious to me is that MAYBE_UNUSED is not
really adding some "maybe_unused" attribute -- but an "unused" one.
If that's fine to add an unused attribute to a function, then
everything's fine.
--
((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:
http://barzilay.org/ Maze is Life!
_________________________________________________
For list-related administrative tasks:
http://list.cs.brown.edu/mailman/listinfo/plt-dev
