On December 14, 2020 9:53 PM, Peff wrote:
> 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.
Strangely, we do have setitimer() on NonStop. I'm not sure how we get into this situation, unless it's not being detected correctly.
> > > 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.
FYI: We successfully built 2.29.2 without issues. I'm not sure how this all happened but seems like it was at 1201eb628a.
Thanks,
Randall
