Powered by Linux
Re: [PATCH] slist: Properly handle one-way merges — Semantic Matching Tool

Re: [PATCH] slist: Properly handle one-way merges

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello!

   I am bringing this up again since it's still a problem.
   I have this memory of you sending an alternative patch that did not clone the whole state,
   but instead just updated the line number for the derivative.

   Anyway, neither one made it to the tree and I think this is better be fixed one way or the other?
   What do you think?

   Thanks.

Bye,
    Oleg
On Feb 15, 2016, at 8:59 AM, Dan Carpenter wrote:

> I'm having a hard time getting the lustre sources to build on debian.
> I'm just getting:
> 
> $ make
> make  all-recursive
> make[1]: Entering directory '/home/dcarpenter/progs/audit/lustre/lustre'
> Making all in ldiskfs
> make[2]: Entering directory '/home/dcarpenter/progs/audit/lustre/lustre/ldiskfs'
> make[2]: *** No rule to make target '../ldiskfs/kernel_patches/series/ldiskfs-', needed by 'sources'.  Stop.
> make[2]: Leaving directory '/home/dcarpenter/progs/audit/lustre/lustre/ldiskfs'
> autoMakefile:587: recipe for target 'all-recursive' failed
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory '/home/dcarpenter/progs/audit/lustre/lustre'
> autoMakefile:483: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> On Sat, Feb 13, 2016 at 11:33:04PM -0500, green@xxxxxxxxxxxxxx wrote:
>> From: Oleg Drokin <green@xxxxxxxxxxxxxx>
>> 
>> When we have a merged state that consists of a single state only,
>> e.g. because both true and false states are the same since the condition
>> did not affect any of them - just clone one of the original states
>> instead of creating a new one and assigning a current line number to it.
> 
> If it's just a matter of merging the line numbers then that's simple
> enough...  But this patch does a bunch of other stuff as well.
> 
>> 
>> This fixes an annoying problem where "xxx was used before see yyy" prints
>> yyy line number that dates back to previous condition, not to actual usage.
> 
> Fair enough.
> 
>> 
>> Additionally when dealing with "ghost" merges this fixes some false
>> positives about lock imbalances for the case of
> 
> The ghost merge code is dead code though.  It seemed like a good idea
> and still does but I forgot to commit a bit of code and then in testing
> there was an issue and I never got around to debugging it so I just
> left the dead code as-is.
> 
>> 
>> if (condition)
>> 	lock(x);
>> some stuff;
>> if (some other stuff)
>> 	goto out;
>> 
>> out:
>> if (condition)
>> 	unlock(x)
>> 
>> Signed-off-by: Oleg Drokin <green@xxxxxxxxxxxxxx>
>> ---
>> 
>> I was long annoyed by this and so using my lustre tree as benchmark
>> here is the difference in output before and after the patch for this file
>> for wrong line numbers:
>> http://git.whamcloud.com/fs/lustre-release.git/blob/943612ac470a1f916d44f551eee01c3d51fc9546:/lustre/lfsck/lfsck_layout.c
>> 
>> -lustre/lfsck/lfsck_layout.c:2390 lfsck_layout_recreate_lovea() warn: variable dereferenced before check 'handle' (see line 2386)
>> -lustre/lfsck/lfsck_layout.c:2415 lfsck_layout_recreate_lovea() warn: variable dereferenced before check 'handle' (see line 2411)
>> +lustre/lfsck/lfsck_layout.c:2390 lfsck_layout_recreate_lovea() warn: variable dereferenced before check 'handle' (see line 2242)
>> +lustre/lfsck/lfsck_layout.c:2415 lfsck_layout_recreate_lovea() warn: variable dereferenced before check 'handle' (see line 2242)
>> 
>> Granted, this still remains a false positive, but thta would need a more
>> involved fix in this particular check module.
>> 
>> And here's the false positive that is now disappeared for this file:
>> http://git.whamcloud.com/fs/lustre-release.git/blob/943612ac470a1f916d44f551eee01c3d51fc9546:/lustre/llite/llite_lib.c
>> i
>> lustre/llite/llite_lib.c:1709 ll_setattr_raw() warn: 'mutex:&inode->i_mutex' is sometimes locked here and sometimes unlocked.
> 
> The most recent Smatch is supposed to get this right.  :(
> 
> I don't know what's going on here.
> 
> regards,
> dan carpenter
> 
> --
> To unsubscribe from this list: send the line "unsubscribe smatch" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux