Re: mark parsing in fast-import

From: Shawn O. Pearce <spearce@spearce.org>
Date: Apr 21, 2008, 23:59 UTC
Message-ID: <20080421235940.GQ29771@spearce.org>
In-Reply-To: <20080421084151.GA31344@alea.gnuu.de>

Jrg Sommer <joerg@alea.gnuu.de> wrote:

Show 5 quoted lines

> Shawn O. Pearce schrieb am Sun 20. Apr, 20:26 (-0400):
> > Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > +static inline int parse_mark(const const char *str, uintmax_t* mark,
> 
> Is inline okay?

Yea, inline is fine.  We use "static inline" often in Git when it
is a good idea.

Show 16 quoted lines

> > >  static void cmd_mark(void)
> > >  {
> > > -	if (!prefixcmp(command_buf.buf, "mark :")) {
> > > -		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
> > > +	uintmax_t mark = 0;
> > > +	char *after_mark = NULL;
> > > +
> > > +	if (!prefixcmp(command_buf.buf, "mark ") &&
> > > +		parse_mark(&command_buf.buf[5], &mark, &after_mark) &&
> > 
> > Hmm.  Shouldn't this be ! parse_mark given that it returns 0
> > on success and 1 on failure?
> 
> Yes, you're right. I've checked some other functions and found this
> behaviour. Can I use a different behabiour, i.e. return 0 on failure and
> !0 on success?

I wasn't objected to the return values as written, but more to the fact that it seemed like a logic error to me. We use both patterns in Git. Perhaps the best example to follow is get_sha1_hex(); it returns -1 on error and 0 on success. So a common pattern is "!get_sha1_hex()" to ensure a successful conversion of a hex string to an unsigned char array.

-- 
Shawn.