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.
-Peff
