Re: [PATCH v2 1/4] sha1_name: add get_sha1_with_context()

[Jeff King <peff@peff.net>]

On Wed, Jun 09, 2010 at 12:30:31AM +0200, Clément Poulain wrote:

> Le 8 juin 2010 19:57, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :
> > This patch produces uncompilable code for me:
> >
> > cc1: warnings being treated as errors
> > In file included from builtin.h:6,
> >                 from fast-import.c:147:
> > cache.h: In function ‘get_sha1_with_context’:
> > cache.h:748: error: implicit declaration of function ‘get_sha1_with_context_1’
> >
> > Forgot to add get_sha1_with_context_1 to cache.h?
> 
> Uh, we compiled it almost ten times on both our pc and ensibm (our
> school server), whithout any problems. Seems that we need to check our
> compilation configurations.

Note the "warnings being treated as errors". Matthieu is compiling with
-Werror (and presumably -Wall). We strive to be warning-free in git, and
I think many of the developers compile with "-Wall -Werror".

> Right. But the aim was to skip one function call (see the call-stack below)
> _with_mode => _with_mode_1 => _with_context_1
> whereas:
>  _with_context => _with_context_1

Perhaps that was your goal, but my goal when I suggested it was to give
us a cleaner codebase. We don't want a proliferation of get_sha1_with_*
functions. Introducing _with_context instead of _with_tree or _with_path
was meant not to make things worse. But collapsing _with_mode into
_with_context actively makes things better.

> >> +                     orc->path[sizeof(orc->path)] = '\0';
> >
> > Same here.
> 
> That's true. Stupid error, we copied this line without checking it.

Oops, that's my fault for introducing the bug in the first place (I had
originally had an snprintf and changed it to strncpy at the last
minute). :)

-Peff