Opened 8 years ago

Closed 7 years ago

#6196 closed task (fixed)

Check for foo = foo; assignments.

Reported by: Kurt Schwehr Owned by: Kurt Schwehr
Priority: normal Milestone:
Component: default Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

It seems crazy that compilers don't warn about this. Even found one that I did in the rik driver: r31300. It would be good to somehow check the codebase for this.

Change History (9)

comment:1 by Kurt Schwehr, 8 years ago

gcc has -Winit-self which seems weaker than clang's -Wself-assign

comment:3 by Even Rouault, 8 years ago

Here's a dummy script that should detect most issues:

import sys

f = open(sys.argv[1], "rb")
lines = f.readlines()
for i in range(len(lines)):
    line = lines[i]
    if len(line) > 0 and line[len(line)-1] == '\n':
        line = line[0:-1]
    tab = line.split('=')
    if len(tab) != 2:
        continue
    left = tab[0].strip()
    right = tab[1].strip()
    if len(right) > 0 and right[len(right)-1] == ';':
        right = right[0:-1]
    else:
        continue
    right = right.strip()
    if left == right:
        print("%s: %d: %s" % (sys.argv[1], i+1, line))

"find frmts -name "*.cpp" -exec python self-assignment.py {} \;" found:

frmts/raw/landataset.cpp: 182:     poDS = poDS;
frmts/grib/degrib18/degrib/tdlpack.cpp: 3410:                G1.start = G1.start;

comment:4 by Even Rouault, 8 years ago

Hum seems you get used to that pattern ;-) After latest svn update :

$ python self-assignment.py frmts/zmap/zmapdataset.cpp
frmts/zmap/zmapdataset.cpp: 102:     nBand = nBand;
Last edited 8 years ago by Even Rouault (previous) (diff)

comment:5 by Kurt Schwehr, 8 years ago

At least my removal of this-> made the issue more obvious to the script. sigh

comment:6 by Kurt Schwehr, 8 years ago

zmap in trunk... r31327. Sadly, the zmap test is just creating a copy from a tif.

It would be good to get more zmap examples. There is one in the spec email from david.m.baker at chk.com:

!
! File created by DMBTools2.GridFileFormats.ZmapPlusFile
!
@GRID FILE, GRID, 4
20, -9999.0000000, , 7, 1
6, 4, 0, 200, 0, 300
0.0, 0.0, 0.0
@
       -9999.0000000       -9999.0000000           3.0000000          32.0000000
          88.0000000          13.0000000
       -9999.0000000          20.0000000           8.0000000          42.0000000
          75.0000000           5.0000000
           5.0000000         100.0000000          35.0000000          50.0000000
          27.0000000           1.0000000
           2.0000000          36.0000000          10.0000000           6.0000000
           9.0000000       -9999.0000000

But it would be good to have more. Can it have multiple bands? What will gdal do if it gets a zmap file with points and polygons?

comment:7 by Even Rouault, 8 years ago

Interestingly, on the following example:

class Aparent
{
    public:
        int foo;
};

class A: public Aparent
{
    public:
        A();
        int bar;
};

A::A()
{
    foo = foo;
    bar = bar;
}

$ /home/even/install-llvm-3.4/bin/clang -c test.cpp

test.cpp:17:9: warning: assigning field to itself [-Wself-assign-field]
    bar = bar;
        ^
1 warning generated.

So it fails to detect self assignment to members of parent class, which was the case for the poDS issues. Same for clang 3.7 by the way.

comment:8 by Even Rouault, 8 years ago

trunk r32069 "frmts/grib/degrib18/degrib/tdlpack.cpp: remove self assignment (#6196)"

comment:9 by Even Rouault, 7 years ago

Resolution: fixed
Status: newclosed

In 36742:

Add scripts/detect_self_assignment.sh and use it in CI (fixes #6196)

Note: See TracTickets for help on using tickets.