[Dovecot] CVS to Mercurial switch

[Timo Sirainen]

On Sat, 2007-05-19 at 22:31 +0300, Timo Sirainen wrote:
> > or Git?
> 
> It seems a bit kludgy with all of its different commands and scripts.
> Also I don't really like its code. It's using standard C functions for
> string manipulations and in general it's using a lot with fixed size
> buffers. If it wasn't written by kernel developers, I'd say it's most
> likely full of buffer overflows. But since it is (was?), perhaps there
> are only a few. I don't want to risk it.

Just out of curiosity I checked it. There are even basic string
manipulation errors in their code:

~/src/git-1.5.1% grep +=.*snprintf *.c
builtin-grep.c:                         len += snprintf(argptr, sizeof(randarg)-len,
builtin-grep.c:                         len += snprintf(argptr, sizeof(randarg)-len,
builtin-grep.c:                 len += snprintf(argptr, sizeof(randarg)-len,
commit.c:               i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
commit.c:               i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
diff.c:         len += snprintf(msg + len, sizeof(msg) - len,
diff.c:         len += snprintf(msg + len, sizeof(msg) - len,
diff.c:                 len += snprintf(msg + len, sizeof(msg) - len,
diff.c:         len += snprintf(msg + len, sizeof(msg) - len,
diff.c:                 len += snprintf(msg + len, sizeof(msg) - len,
diff.c:         len += snprintf(msg + len, sizeof(msg) - len, "\n");
path.c: len += vsnprintf(pathname + len, PATH_MAX - len, fmt, args);

Every single one of those is wrong. Linux kernel's snprintf() handles
code like this safely, but libc doesn't.

Possibly other snprintf() uses are wrong as well.

strncpy() then:
~/src/git-1.5.1% grep strncpy *.c    
builtin-log.c:                          strncpy(ref_message_id,
message_id,
builtin-shortlog.c:             strncpy(name, realname, maxlen);
diff-lib.c:                             strncpy(buffer1 + len1,
p1.items[i1++].path,
diff-lib.c:                             strncpy(buffer2 + len2,
p2.items[i2++].path,
fast-import.c:  strncpy(c->str_dat, s, len);
fast-import.c:  strncpy(ident, buf, name_len);
interpolate.c:                                  strncpy(dest, value,
valuelen);
trace.c:        strncpy(trace_str, format_str, format_len);
trace.c:        strncpy(trace_str + format_len, argv_str, argv_len);

correct, wrong, wrong, wrong, correct, correct

The last 3 are just stupid. They should be memcpy()s to make it clear
what they intend to do.

I don't even want to start verifying the correctness of sprintf() and
strcpy() calls.

Wonder if I should have Cc'd this to git list. Oh well, someone else can
if they care.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://dovecot.org/pipermail/dovecot/attachments/20070519/f13a8217/attachment.pgp