About some bugs

[bearophile]

I have studied more Linux bugs.

----------------

An example of bug (more than 14 like this fixed in few years):

-       memset(pp, 0, sizeof(pp));
+       memset(pp, 0, sizeof(*pp));

-       memcpy((caddr_t)TstSchedTbl, (caddr_t)&vcIndex,sizeof(TstSchedTbl));
+       memcpy((caddr_t)TstSchedTbl, (caddr_t)&vcIndex, sizeof(*TstSchedTbl));

Here the type system knows that pp is a pointer. sizeof(pp) is typically a word, while the correct sizeof(*pp) is often larger. A simple way to avoid this bug in D is to use a zerioing template function, something like (untested) (in GNU C there is a way to write a similar macro, I don't know why they don't use it, even if it's a bit less safe and much less nice looking):

void zeroit(T)(T* ptr) if (!IsPointer!T) {
    memset(ptr, 0, (*ptr).sizeof);
}

Standard safer wrappers for some C functions may help low-level D coding.

If you don't want to use a zeroit() then a type system able to catch such bugs needs some nice annotations...

----------------

A common (> 12 cases in few years) example of redundant code:

        u32 da = new->da_start;
-       if (!obj || !new || !sgt) {...
+       if (!obj || !sgt) {...

If the first line is correct, then "new" can't be NULL, so there's no need to test "|| !new".

In 13 similar cases the code is inside a branch where the NULL test of the pointer is already done:

if (ptr != NULL) {
    if (!ptr) { ...
}

----------------

In 7 cases the result of malloc-like function was not tested for NULL:

        agp = kmalloc(sizeof(*agp), GFP_KERNEL);
+       if (!agp)
+               return NULL;

----------------

A very common case (20 cases in few years) are like this, where a pointer is deferenced before the NULL test:

        block = bdev->bd_disk->private_data;
-       base = block->base;
        if (!block)
                return -ENODEV;
+       base = block->base;

----------------

2 cases like this (the pattern (!E && !E->fld) is nonsensical):

-               if (!slot->hotplug_slot && !slot->hotplug_slot->info) 
+               if (!slot->hotplug_slot || !slot->hotplug_slot->info)
                        continue;

----------------

There are 11 cases like this (alloc_bootmem never returns NULL and it always sets the momory to zero):

    zero_page = alloc_bootmem_low_pages_node(NODE_DATA(0), PAGE_SIZE);
-   memset(zero_page, 0, PAGE_SIZE);

----------------

There are a lot of cases (34) of missing free:

+       kfree(iter);

----------------

Two cases need a wider NULL test:

-               if (fep != NULL) {
+               if (fep && fep->ops) {
                        (*fep->ops->free_bd)(ndev);
                        (*fep->ops->cleanup_data)(ndev);
                }

----------------

In this post I don't see any little rule worth adding to the D compiler. So to bugzilla I will add only the !x&y thing.

But from what I have seen many bugs are NULL-related, and many of them are avoidable with two features present at the same time in the language:

1) Nonnull pointer/reference tag (with it you don't miss some null tests, and you avoid some redundant tests);

2) Plus a bit of inter-function (not intra, for that nonnull signatures are enough) flow analysis to avoid bugs like:

base = block->base;
if (!block)
    return -ENODEV;

if (fep != NULL) {
    (*fep->ops->free_bd)(ndev);

(P.S.: integer overflow bugs happen in Linux kernel too.)

Bye,
bearophile