* re: btrfs: initial readahead code and prototypes
@ 2012-05-17 7:14 Dan Carpenter
2012-05-17 13:31 ` Arne Jansen
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-05-17 7:14 UTC (permalink / raw)
To: linux-btrfs; +Cc: sensille
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: btrfs: initial readahead code and prototypes
2012-05-17 7:14 btrfs: initial readahead code and prototypes Dan Carpenter
@ 2012-05-17 13:31 ` Arne Jansen
2012-05-17 13:46 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Arne Jansen @ 2012-05-17 13:31 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-btrfs
On 05/17/12 09:14, Dan Carpenter wrote:
> 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.
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
>
> 148 /*
> 149 * FIXME: currently we just set nritems to 0 if this is a leaf,
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: btrfs: initial readahead code and prototypes
2012-05-17 13:31 ` Arne Jansen
@ 2012-05-17 13:46 ` Dan Carpenter
2012-05-17 15:33 ` Arne Jansen
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-05-17 13:46 UTC (permalink / raw)
To: Arne Jansen; +Cc: linux-btrfs
On Thu, May 17, 2012 at 03:31:50PM +0200, Arne Jansen wrote:
> 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.
> This code changes significantly with the patch
>
> btrfs: extend readahead interface
>
> Where it is written in a more obvious way.
Cool.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: btrfs: initial readahead code and prototypes
2012-05-17 13:46 ` Dan Carpenter
@ 2012-05-17 15:33 ` Arne Jansen
0 siblings, 0 replies; 4+ messages in thread
From: Arne Jansen @ 2012-05-17 15:33 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-btrfs
On 05/17/12 15:46, Dan Carpenter wrote:
> On Thu, May 17, 2012 at 03:31:50PM +0200, Arne Jansen wrote:
>> 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
>
>> This code changes significantly with the patch
>>
>> btrfs: extend readahead interface
>>
>> Where it is written in a more obvious way.
>
> Cool.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-05-17 15:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-17 7:14 btrfs: initial readahead code and prototypes Dan Carpenter
2012-05-17 13:31 ` Arne Jansen
2012-05-17 13:46 ` Dan Carpenter
2012-05-17 15:33 ` Arne Jansen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).