git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [ANNOUNCE] git-2.30.0-rc0 - Compile Fails on HPE NonStop

From
Elijah Newren <newren@gmail.com>
Date
Dec 15, 2020, 16:21 UTC
Message-ID
<CABPp-BEnkQahdVJP+rfQho+S_hcS3y-o3cZBUMhU-EQHjJTLOQ@mail.gmail.com>
In-Reply-To
<X9glBJnlLMTW6Dns@coredump.intra.peff.net>
On Mon, Dec 14, 2020 at 6:52 PM Jeff King <peff@peff.net> wrote:
Show 66 quoted lines
>
> On Mon, Dec 14, 2020 at 06:43:36PM -0800, Junio C Hamano wrote:
>
> > diff --git c/git-compat-util.h w/git-compat-util.h
> > index 7d509c5022..58cd0761be 100644
> > --- c/git-compat-util.h
> > +++ w/git-compat-util.h
> > @@ -273,7 +273,8 @@ struct itimerval {
> >
> >  #ifdef NO_SETITIMER
> >  static inline int setitimer(int which, const struct itimerval *value, struct itimerval *newvalue) {
> > -     ; /* nothing */
> > +     errno = ENOSYS;
> > +     return -1; /* not implemented */
> >  }
> >  #endif
> >
> > Alternatively we could pretend that the call always succeeds by
> > without touching errno and returning 0.  That might be safer, but I
> > dunno which one we want, and I do not have a system affected by the
> > choice.
>
> I think this is a sensible choice. Before the conversion to an inline,
> the code was removed entirely! So anybody checking the return value
> would have seen an error, and we do not have to worry much about
> breaking them.
>
> For new callers, anybody checking the return value would probably
> appreciate the warning that support for the function is optional (OTOH,
> they would probably not find out themselves, but rather when Randall
> tells them ;) ).
>
> It would be nice to have a way to warn them even on platforms that have
> setitimer(), but I can't think of an easy way.
>
> > > Aside from inlining bodies, this should not have compiled on any platform:
> > >
> > > static inline void strset_remove(struct strset *set, const char *str)
> > > {
> > >         return strmap_remove(&set->map, str, 0);
> > > }
> > >
> > > What is really intended here?
> >
> > I think we should just drop "return"; a void function should be
> > called in void context without requiring a value, even if that
> > return expects no value.
>
> Yeah, I think that is right. I checked earlier iterations of the series
> to see if perhaps strmap_remove() had previously returned a value, but
> it never did in any on the list.
>
> > diff --git i/strmap.h w/strmap.h
> > index c4c104411b..1e152d832d 100644
> > --- i/strmap.h
> > +++ w/strmap.h
> > @@ -165,7 +165,7 @@ static inline int strintmap_contains(struct strintmap *map, const char *str)
> >
> >  static inline void strintmap_remove(struct strintmap *map, const char *str)
> >  {
> > -     return strmap_remove(&map->map, str, 0);
> > +     strmap_remove(&map->map, str, 0);
> >  }
>
> So yeah, I think that is the right fix. +cc Elijah for any other
> insight.

Doh, sorry for the bug. Yeah, that's the exact fix I'd make. Junio, do you want to just use the changes you've already got, or would you like me to submit a patch removing these two 'return's?

Previous: Randall S. BeckerNext: Junio C Hamano
Message 5 of 12 in “Re: [ANNOUNCE] git-2.30.0-rc0 - Compile Fails on HPE NonStop”
  1. Randall S. BeckerDec 14, 2020
  2. Junio C HamanoDec 15, 2020
  3. Jeff KingDec 15, 2020
  4. Randall S. BeckerDec 15, 2020
  5. Elijah NewrenDec 15, 2020
  6. Junio C HamanoDec 15, 2020
  7. Junio C HamanoDec 15, 2020
  8. strmap: make callers of strmap_remove() to call it in void contextJunio C Hamano, Dec 15, 2020
  9. Randall S. BeckerDec 15, 2020
  10. compat-util: pretend that stub setitimer() always succeedsJunio C Hamano, Dec 15, 2020
  11. Jeff KingDec 15, 2020
  12. Junio C HamanoDec 16, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.