* re: Btrfs: add optional integrity check code
@ 2014-05-09 12:00 Dan Carpenter
2014-05-09 13:02 ` Stefan Behrens
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2014-05-09 12:00 UTC (permalink / raw)
To: sbehrens; +Cc: linux-btrfs
Hello Stefan Behrens,
The patch 5db0276014b8: "Btrfs: add optional integrity check code"
from Nov 1, 2011, leads to the following static checker warning:
fs/btrfs/check-integrity.c:1099 btrfsic_process_metablock()
warn: missing error code here? 'btrfsic_stack_frame_alloc()' failed. 'sf->error' = '0'
fs/btrfs/check-integrity.c
1092
1093 next_stack =
1094 btrfsic_stack_frame_alloc();
1095 if (NULL == next_stack) {
1096 btrfsic_release_block_ctx(
1097 &sf->
1098 next_block_ctx);
Should we set "sf->error" here? I don't know the code well enough to
say the answer.
1099 goto one_stack_frame_backwards;
1100 }
1101
1102 next_stack->i = -1;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: Btrfs: add optional integrity check code
2014-05-09 12:00 Btrfs: add optional integrity check code Dan Carpenter
@ 2014-05-09 13:02 ` Stefan Behrens
0 siblings, 0 replies; 2+ messages in thread
From: Stefan Behrens @ 2014-05-09 13:02 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-btrfs
On Fri, 9 May 2014 15:00:07 +0300, Dan Carpenter wrote:
> Hello Stefan Behrens,
>
> The patch 5db0276014b8: "Btrfs: add optional integrity check code"
> from Nov 1, 2011, leads to the following static checker warning:
>
> fs/btrfs/check-integrity.c:1099 btrfsic_process_metablock()
> warn: missing error code here? 'btrfsic_stack_frame_alloc()' failed. 'sf->error' = '0'
>
> fs/btrfs/check-integrity.c
> 1092
> 1093 next_stack =
> 1094 btrfsic_stack_frame_alloc();
> 1095 if (NULL == next_stack) {
> 1096 btrfsic_release_block_ctx(
> 1097 &sf->
> 1098 next_block_ctx);
>
> Should we set "sf->error" here? I don't know the code well enough to
> say the answer.
>
> 1099 goto one_stack_frame_backwards;
> 1100 }
> 1101
> 1102 next_stack->i = -1;
Looking at this function immediately made me blind and getting terrible
headache, therefore I can only guess whether not setting sf->error was
intentional or not three years ago. Nowadays, I'd set sf->error since
this propagates the error condition upwards. Although it doesn't make a
difference to the user and doesn't cause a crash whether it is set or
not. But if the function returns an error/success status, and an error
was detected, the error status should be returned. I'll send a patch.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-05-09 13:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-09 12:00 Btrfs: add optional integrity check code Dan Carpenter
2014-05-09 13:02 ` Stefan Behrens
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).