Re: mark parsing in fast-import

Jrg Sommer [off-list ref] wrote:

Shawn O. Pearce schrieb am Sun 20. Apr, 20:26 (-0400):

quoted

Jrg Sommer [off-list ref] wrote:

quoted

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

quoted

quoted

 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.