Re: Splint report and scripts for gdb (was: Expand bitpos and type.length)

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Thu, 28 Jun 2012 15:18:14 +0200, Siddhesh Poyarekar wrote:
> After all the sanitization, what we have left is a very clean pattern,
> which is:
> 
> LOC (file:num)
>   The warning for this LOC

I have read it etc. and I agree.

[...]
> http://siddhesh.fedorapeople.org/splint-bitpos.tar.xz
> 
> These are the exact same files that you had uploaded earlier, so you
> should be able to git-init a repository from your files and then
> overwrite them with my copy to get a difference.
> 
> Can you please review the script changes?

You have made '^ LOC' and '^   LOC' equal but they are not by the changes:
-1 while $f=~s{^ LOC .*\n(?: .*?\n)*?(.LOC )}{$1}gm;
+1 while $f=~s{^\s+LOC .*\n(?: .*?\n)*?((.\s*LOC )|(.Finished checking))}{$1}gm;
+# Finally, remove the extra spaces from the beginning of the LOC lines.
+$f=~s{^\s+(LOC.*\n)}{$1}gm;

'^ LOC' always starts a new splint record while '^   LOC' (more spaces) is
only supplementary information for that record.  See this single original
record:
ada-lang.c:6510: Fresh storage type not released before return
   ada-lang.c:6478: Fresh storage type created

Translated into:
LOC ada-lang.c:6492
 Fresh storage type not released before return
   LOC ada-lang.c:6460
 Fresh storage type created

While you now consider it as two different records.  But this should not miss
any record, it just adds more records to process - although you discard these
in splint-siddhesh-process-locdiff by rules like so it does not matter:
        if (/Previous definition of \w+: \w+/) {
Not sure if it was all intentional or not.

        # If the TYPE_LENGTH expression is a builtin_type then we're safe.
        $_ = $expr;
        return 1 if (/builtin_type[^ ]+->length/);
 - I would not call it safe, it can be a part of an arbitrary large expression.
   But the 6 cases where it applies are OK.

        if (/^ (Right)|(Left) operand of \W+ /) {
 - it did not discard any important line but the regex is dangerously wrong, it
   does:
        if (/^ Right/ || /Left operand of \W+ /) {


Otherwise I have made comments to your .report file.  I believe we should
follow now the 'compact' form instead.  It is a lot of work as I see.


Thanks,
Jan

Attachment: splint-bitpos.locdiff.report.diff
Description: Text document