Skip to contentSkip to conversation

Btrfs: add optional integrity check code

DISCUSSION Dan Carpenter 2 participants 2 messages May 9, 2014 Btrfs file system

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 17 lines · Dan Carpenter

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);

Quoted 7 lines

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.