btrfs: initial readahead code and prototypes
Hi, I'm working on some new Smatch code and it complains about this patch from last year. -Dan
----
This is a semi-automatic email about new static checker warnings.
The patch 7414a03fbf9e: "btrfs: initial readahead code and prototypes" from May 23, 2011, leads to the following Smatch complaint:
fs/btrfs/reada.c:147 __readahead_hook()
error: we previously assumed 'eb' could be null (see line 122)
fs/btrfs/reada.c
121
122 if (eb)
^^^^
Checked here.
123 level = btrfs_header_level(eb);
124
125 /* find extent */
126 spin_lock(&fs_info->reada_lock);
127 re = radix_tree_lookup(&fs_info->reada_tree, index);
128 if (re)
129 kref_get(&re->refcnt);
130 spin_unlock(&fs_info->reada_lock);
131
132 if (!re)
133 return -1;
134
135 spin_lock(&re->lock);
136 /*
137 * just take the full list from the extent. afterwards we
138 * don't need the lock anymore
139 */
140 list_replace_init(&re->extctl, &list);
141 for_dev = re->scheduled_for;
142 re->scheduled_for = NULL;
143 spin_unlock(&re->lock);
144
145 if (err == 0) {
146 nritems = level ? btrfs_header_nritems(eb) : 0;
^^^^^Checked here again indirectly.
147 generation = btrfs_header_generation(eb);
^^^^^^^^^^^^^^^^^^^^^^^^^^^Dereferenced inside function without checking.
148 /*
149 * FIXME: currently we just set nritems to 0 if this is a leaf,
regards,
dan carpenter
Quoted 44 lines · Dan Carpenter
Hi, I'm working on some new Smatch code and it complains about this patch from last year. -Dan
----
This is a semi-automatic email about new static checker warnings.The patch
7414a03fbf9e: "btrfs: initial readahead code and prototypes" from May 23, 2011, leads to the following Smatch complaint:
fs/btrfs/reada.c:147__readahead_hook()
error: we previously assumed 'eb' could be null (see line 122)
fs/btrfs/reada.c121 122 if (eb) ^^^^Checked here.
123 level = btrfs_header_level(eb); 124 125 /* find extent */ 126 spin_lock(&fs_info->reada_lock); 127 re = radix_tree_lookup(&fs_info->reada_tree, index); 128 if (re) 129 kref_get(&re->refcnt); 130 spin_unlock(&fs_info->reada_lock); 131 132 if (!re) 133 return -1; 134 135 spin_lock(&re->lock); 136 /* 137 * just take the full list from the extent. afterwards we 138 * don't need the lock anymore 139 */ 140 list_replace_init(&re->extctl,&list); 141 for_dev = re->scheduled_for; 142 re->scheduled_for = NULL; 143 spin_unlock(&re->lock); 144 145 if (err == 0) { 146 nritems = level ? btrfs_header_nritems(eb) : 0; ^^^^^
Quoted 5 lines
Checked here again indirectly.
147 generation = btrfs_header_generation(eb); ^^^^^^^^^^^^^^^^^^^^^^^^^^^Dereferenced inside function without checking.
The assumption here is that if err == 0, eb is always != NULL. There's even a tiny comment above the function stating this:
107 /* in case of err, eb might be NULL */
This code changes significantly with the patch
btrfs: extend readahead interface
Where it is written in a more obvious way.
-Arne
Quoted 7 lines
148 /*
149 * FIXME: currently we just set nritems to 0 if this is a leaf,regards,
dan carpenter
Quoted 5 lines · Arne Jansen
The assumption here is that if err == 0, eb is always != NULL. There's even a tiny comment above the function stating this:
107 /* in case of err, eb might be NULL */
Ah, right. I missed the comment.
Quoted 5 lines
This code changes significantly with the patch
btrfs: extend readahead interface
Where it is written in a more obvious way.
Cool.
regards,
dan carpenter
Quoted 8 lines · Dan Carpenter
Quoted 5 lines · Arne Jansen
The assumption here is that if err == 0, eb is always != NULL. There's even a tiny comment above the function stating this:
107 /* in case of err, eb might be NULL */
Ah, right. I missed the comment.
Thanks for doing this kind of sanity checking :)
-Arne
Quoted 12 lines
Quoted 6 lines
This code changes significantly with the patch
btrfs: extend readahead interface
Where it is written in a more obvious way.
Cool.
regards,
dan carpenter