potential null dereference
Hi,
Stanse found the following error in unpack-trees.c:
dereferencing NULL pointer here.[. * o src_index]
int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
{
int ret;
static struct cache_entry *dfc;
...
if (o->src_index) { <-- loc0
o->result.timestamp.sec = o->src_index->timestamp.sec; o->result.timestamp.nsec = o->src_index->timestamp.nsec;
}
o->merge_size = len;
if (!dfc)
dfc = xcalloc(1, ((1 + (0) + 8) & ~7));
o->df_conflict_entry = dfc;
if (len) {
...
}
if (o->merge) {
while (o->pos < o->src_index->cache_nr) { <-- here
It triggers, because there is a test for o->src_index being NULL at loc0, but here, it is dereferenced without a check. Can this happen (e.g. does o->merge != NULL imply o->src_index != NULL)?
Further, there is a warning in log-tree.c:
pointer always points to valid memory here, but checking for not NULL.[parents]
static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
{
int showed_log;
struct commit_list *parents;
unsigned const char *sha1 = commit->object.sha1;
if (!opt->diff && !((&opt->diffopt)->flags & (1 << 14)))
return 0;
parents = commit->parents;
if (!parents) { <-- loc0
if (opt->show_root_diff) {
diff_root_tree_sha1(sha1, "", &opt->diffopt);
log_tree_diff_flush(opt);
}
return !opt->loginfo; <-- loc1
}
if (parents && parents->next) { <-- here
I.e. if parents was NULL at loc0, we escaped at loc1. But we check parents against NULL here again.
thanks,
Signature · 1 line
js
Am 15.12.2009 13:41, schrieb Jiri Slaby:
Quoted 25 lines
Hi,
Stanse found the following error in
unpack-trees.c:
dereferencing NULL pointer here.[. * osrc_index]int
unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
{
int ret;
static structcache_entry*dfc;
...
if (o->src_index) { <-- loc0
o->result.timestamp.sec=o->src_index->timestamp.sec;o->result.timestamp.nsec=o->src_index->timestamp.nsec;
}
o->merge_size= len;if (!dfc)
dfc =xcalloc(1, ((1 + (0) + 8) & ~7));
o->df_conflict_entry= dfc;if (len) {
...
}
Quoted 6 lines
if (
o->merge) {
while (o->pos<o->src_index->cache_nr) { <-- hereIt triggers, because there is a test for
o->src_indexbeing NULL at loc0, but here, it is dereferenced without a check. Can this happen (e.g. doeso->merge!= NULL implyo->src_index!= NULL)?
Running "git grep -w -B70 unpack_trees" and looking for "src_index" using less' search command showed me that src_index is never NULL when unpack_trees() is called.
Quoted 20 lines
Further, there is a warning in
log-tree.c:
pointer always points to valid memory here, but checking for not NULL.[parents]static int
log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
{
intshowed_log;
structcommit_list*parents;
unsigned const char *sha1 =commit->object.sha1;if (!opt->diff && !((&opt->diffopt)->flags & (1 << 14)))
return 0;parents =
commit->parents;
if (!parents) { <-- loc0if (opt->show_root_diff) { diff_root_tree_sha1(sha1, "", &opt->diffopt); log_tree_diff_flush(opt);
Quoted 8 lines
}
return !opt->loginfo; <-- loc1
}if (parents &&
parents->next) { <-- hereI.e. if parents was NULL at loc0, we escaped at loc1. But we check parents against NULL here again.
The check may be duplicate, but I suspect removing it won't change the resulting object code -- the compiler should be smart enough to come to the same conclusion.
Thanks,
René