fix Coverity braindamage in UDF
PATCH 1/1 fix Coverity braindamage in UDF
Andrew, please don't blindly apply Coverity patches. While the checker is smart at finding inconsistencies, that "obvious" fix is wrong most of the item. As in this unreviewed UDF patch that got in:udf_find_entry can never be called with a NULL argument, so we shouldn't check for it instead of adding more assignments behind the check.
fs/udf/namei.c +2 -9
@@ -153,24 +153,17 @@ struct fileIdentDesc *cfi) {struct fileIdentDesc *fi=NULL ; -loff_t f_pos; +loff_t f_pos = (udf_ext0_offset (dir) >>2 );int block, flen;char fname[UDF_NAME_LEN ];char *nameptr;uint8_t lfi;uint16_t liu; -loff_t size; +loff_t size = (udf_ext0_offset (dir) + dir->i_size) >>2 ;kernel_lb_addr bloc, eloc;uint32_t extoffset, elen, offset;struct buffer_head *bh =NULL ; -if (!dir) -return NULL ; - - size = (udf_ext0_offset (dir) + dir->i_size) >>2 ; - - f_pos = (udf_ext0_offset (dir) >>2 ); - fibh->soffset = fibh->eoffset = (f_pos & ((dir->i_sb->s_blocksize -1 ) >>2 )) <<2 ;if (UDF_I_ALLOCTYPE (dir) ==ICBTAG_FLAG_AD_IN_ICB ) fibh->sbh = fibh->ebh =NULL ;
Quoted 6 lines · Christoph Hellwig
Andrew, please don't blindly apply Coverity patches. While the checker is smart at finding inconsistencies, that "obvious" fix is wrong most of the item. As in this unreviewed UDF patch that got in:
udf_find_entrycan never be called with a NULL argument, so we shouldn't check for it instead of adding more assignments behind the check.
I reviewed it. The code as it stood was wrong and the patch corrected it.
Yes, I realised at the time that the test for null is probably redundant, but that's a separate thing - we have many such redundant tests and perhaps someone should do a cleanup sweep. But I have no intention of doing that and I don't want to be leaving obvious coding errors in there.
A similar argument applies to the patch "coverity: tty_ldisc_ref return null check".