Skip to content

Commit 57dafd3

Browse files
owen-mcowen-mc
committed
Improve test for UnhandledCloseWritableHandle
Now the different paths won't have the same two sources.
1 parent f05c862 commit 57dafd3

File tree

2 files changed

+62
-74
lines changed

2 files changed

+62
-74
lines changed

‎go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/UnhandledCloseWritableHandle.expected

Lines changed: 40 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,46 @@
11
edges
2-
| tests.go:9:2:9:74 | ... := ...[0] | tests.go:10:9:10:9 | f |
3-
| tests.go:10:9:10:9 | f | tests.go:43:5:43:38 | ... := ...[0] |
4-
| tests.go:10:9:10:9 | f | tests.go:60:5:60:38 | ... := ...[0] |
5-
| tests.go:10:9:10:9 | f | tests.go:108:5:108:38 | ... := ...[0] |
6-
| tests.go:10:9:10:9 | f | tests.go:124:5:124:38 | ... := ...[0] |
7-
| tests.go:18:2:18:69 | return statement[0] | tests.go:53:5:53:42 | ... := ...[0] |
8-
| tests.go:18:2:18:69 | return statement[0] | tests.go:70:5:70:42 | ... := ...[0] |
9-
| tests.go:21:24:21:24 | definition of f | tests.go:22:8:22:8 | f |
10-
| tests.go:25:32:25:32 | definition of f | tests.go:26:13:26:13 | capture variable f |
11-
| tests.go:26:13:26:13 | capture variable f | tests.go:27:3:27:3 | f |
12-
| tests.go:43:5:43:38 | ... := ...[0] | tests.go:44:21:44:21 | f |
13-
| tests.go:43:5:43:38 | ... := ...[0] | tests.go:45:29:45:29 | f |
14-
| tests.go:44:21:44:21 | f | tests.go:21:24:21:24 | definition of f |
15-
| tests.go:45:29:45:29 | f | tests.go:25:32:25:32 | definition of f |
16-
| tests.go:53:5:53:42 | ... := ...[0] | tests.go:54:21:54:21 | f |
17-
| tests.go:53:5:53:42 | ... := ...[0] | tests.go:55:29:55:29 | f |
18-
| tests.go:54:21:54:21 | f | tests.go:21:24:21:24 | definition of f |
19-
| tests.go:55:29:55:29 | f | tests.go:25:32:25:32 | definition of f |
20-
| tests.go:60:5:60:38 | ... := ...[0] | tests.go:62:3:62:3 | f |
21-
| tests.go:70:5:70:42 | ... := ...[0] | tests.go:72:3:72:3 | f |
22-
| tests.go:108:5:108:38 | ... := ...[0] | tests.go:110:9:110:9 | f |
23-
| tests.go:124:5:124:38 | ... := ...[0] | tests.go:128:3:128:3 | f |
2+
| tests.go:8:24:8:24 | definition of f | tests.go:9:8:9:8 | f |
3+
| tests.go:12:32:12:32 | definition of f | tests.go:13:13:13:13 | capture variable f |
4+
| tests.go:13:13:13:13 | capture variable f | tests.go:14:3:14:3 | f |
5+
| tests.go:31:5:31:78 | ... := ...[0] | tests.go:32:21:32:21 | f |
6+
| tests.go:31:5:31:78 | ... := ...[0] | tests.go:33:29:33:29 | f |
7+
| tests.go:32:21:32:21 | f | tests.go:8:24:8:24 | definition of f |
8+
| tests.go:33:29:33:29 | f | tests.go:12:32:12:32 | definition of f |
9+
| tests.go:43:5:43:76 | ... := ...[0] | tests.go:44:21:44:21 | f |
10+
| tests.go:43:5:43:76 | ... := ...[0] | tests.go:45:29:45:29 | f |
11+
| tests.go:44:21:44:21 | f | tests.go:8:24:8:24 | definition of f |
12+
| tests.go:45:29:45:29 | f | tests.go:12:32:12:32 | definition of f |
13+
| tests.go:51:5:51:78 | ... := ...[0] | tests.go:53:3:53:3 | f |
14+
| tests.go:63:5:63:76 | ... := ...[0] | tests.go:65:3:65:3 | f |
15+
| tests.go:105:5:105:78 | ... := ...[0] | tests.go:107:9:107:9 | f |
16+
| tests.go:122:5:122:78 | ... := ...[0] | tests.go:126:3:126:3 | f |
2417
nodes
25-
| tests.go:9:2:9:74 | ... := ...[0] | semmle.label | ... := ...[0] |
26-
| tests.go:10:9:10:9 | f | semmle.label | f |
27-
| tests.go:18:2:18:69 | return statement[0] | semmle.label | return statement[0] |
28-
| tests.go:21:24:21:24 | definition of f | semmle.label | definition of f |
29-
| tests.go:22:8:22:8 | f | semmle.label | f |
30-
| tests.go:25:32:25:32 | definition of f | semmle.label | definition of f |
31-
| tests.go:26:13:26:13 | capture variable f | semmle.label | capture variable f |
32-
| tests.go:27:3:27:3 | f | semmle.label | f |
33-
| tests.go:43:5:43:38 | ... := ...[0] | semmle.label | ... := ...[0] |
18+
| tests.go:8:24:8:24 | definition of f | semmle.label | definition of f |
19+
| tests.go:9:8:9:8 | f | semmle.label | f |
20+
| tests.go:12:32:12:32 | definition of f | semmle.label | definition of f |
21+
| tests.go:13:13:13:13 | capture variable f | semmle.label | capture variable f |
22+
| tests.go:14:3:14:3 | f | semmle.label | f |
23+
| tests.go:31:5:31:78 | ... := ...[0] | semmle.label | ... := ...[0] |
24+
| tests.go:32:21:32:21 | f | semmle.label | f |
25+
| tests.go:33:29:33:29 | f | semmle.label | f |
26+
| tests.go:43:5:43:76 | ... := ...[0] | semmle.label | ... := ...[0] |
3427
| tests.go:44:21:44:21 | f | semmle.label | f |
3528
| tests.go:45:29:45:29 | f | semmle.label | f |
36-
| tests.go:53:5:53:42 | ... := ...[0] | semmle.label | ... := ...[0] |
37-
| tests.go:54:21:54:21 | f | semmle.label | f |
38-
| tests.go:55:29:55:29 | f | semmle.label | f |
39-
| tests.go:60:5:60:38 | ... := ...[0] | semmle.label | ... := ...[0] |
40-
| tests.go:62:3:62:3 | f | semmle.label | f |
41-
| tests.go:70:5:70:42 | ... := ...[0] | semmle.label | ... := ...[0] |
42-
| tests.go:72:3:72:3 | f | semmle.label | f |
43-
| tests.go:108:5:108:38 | ... := ...[0] | semmle.label | ... := ...[0] |
44-
| tests.go:110:9:110:9 | f | semmle.label | f |
45-
| tests.go:124:5:124:38 | ... := ...[0] | semmle.label | ... := ...[0] |
46-
| tests.go:128:3:128:3 | f | semmle.label | f |
29+
| tests.go:51:5:51:78 | ... := ...[0] | semmle.label | ... := ...[0] |
30+
| tests.go:53:3:53:3 | f | semmle.label | f |
31+
| tests.go:63:5:63:76 | ... := ...[0] | semmle.label | ... := ...[0] |
32+
| tests.go:65:3:65:3 | f | semmle.label | f |
33+
| tests.go:105:5:105:78 | ... := ...[0] | semmle.label | ... := ...[0] |
34+
| tests.go:107:9:107:9 | f | semmle.label | f |
35+
| tests.go:122:5:122:78 | ... := ...[0] | semmle.label | ... := ...[0] |
36+
| tests.go:126:3:126:3 | f | semmle.label | f |
4737
subpaths
4838
#select
49-
| tests.go:22:8:22:8 | f | tests.go:9:2:9:74 | ... := ...[0] | tests.go:22:8:22:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:9:12:9:74 | call to OpenFile | call to OpenFile |
50-
| tests.go:22:8:22:8 | f | tests.go:18:2:18:69 | return statement[0] | tests.go:22:8:22:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:18:9:18:69 | call to OpenFile | call to OpenFile |
51-
| tests.go:27:3:27:3 | f | tests.go:9:2:9:74 | ... := ...[0] | tests.go:27:3:27:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:9:12:9:74 | call to OpenFile | call to OpenFile |
52-
| tests.go:27:3:27:3 | f | tests.go:18:2:18:69 | return statement[0] | tests.go:27:3:27:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:18:9:18:69 | call to OpenFile | call to OpenFile |
53-
| tests.go:62:3:62:3 | f | tests.go:9:2:9:74 | ... := ...[0] | tests.go:62:3:62:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:9:12:9:74 | call to OpenFile | call to OpenFile |
54-
| tests.go:72:3:72:3 | f | tests.go:18:2:18:69 | return statement[0] | tests.go:72:3:72:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:18:9:18:69 | call to OpenFile | call to OpenFile |
55-
| tests.go:110:9:110:9 | f | tests.go:9:2:9:74 | ... := ...[0] | tests.go:110:9:110:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:9:12:9:74 | call to OpenFile | call to OpenFile |
56-
| tests.go:128:3:128:3 | f | tests.go:9:2:9:74 | ... := ...[0] | tests.go:128:3:128:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:9:12:9:74 | call to OpenFile | call to OpenFile |
39+
| tests.go:9:8:9:8 | f | tests.go:31:5:31:78 | ... := ...[0] | tests.go:9:8:9:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:31:15:31:78 | call to OpenFile | call to OpenFile |
40+
| tests.go:9:8:9:8 | f | tests.go:43:5:43:76 | ... := ...[0] | tests.go:9:8:9:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:43:15:43:76 | call to OpenFile | call to OpenFile |
41+
| tests.go:14:3:14:3 | f | tests.go:31:5:31:78 | ... := ...[0] | tests.go:14:3:14:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:31:15:31:78 | call to OpenFile | call to OpenFile |
42+
| tests.go:14:3:14:3 | f | tests.go:43:5:43:76 | ... := ...[0] | tests.go:14:3:14:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:43:15:43:76 | call to OpenFile | call to OpenFile |
43+
| tests.go:53:3:53:3 | f | tests.go:51:5:51:78 | ... := ...[0] | tests.go:53:3:53:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:51:15:51:78 | call to OpenFile | call to OpenFile |
44+
| tests.go:65:3:65:3 | f | tests.go:63:5:63:76 | ... := ...[0] | tests.go:65:3:65:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:63:15:63:76 | call to OpenFile | call to OpenFile |
45+
| tests.go:107:9:107:9 | f | tests.go:105:5:105:78 | ... := ...[0] | tests.go:107:9:107:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:105:15:105:78 | call to OpenFile | call to OpenFile |
46+
| tests.go:126:3:126:3 | f | tests.go:122:5:122:78 | ... := ...[0] | tests.go:126:3:126:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:122:15:122:78 | call to OpenFile | call to OpenFile |

‎go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/tests.go

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,6 @@ import (
55
"os"
66
)
77

8-
func openFileWrite(filename string) (*os.File, error) {
9-
f, err := os.OpenFile(filename, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666)
10-
return f, err
11-
}
12-
13-
func openFileRead(filename string) (*os.File, error) {
14-
return os.OpenFile(filename, os.O_RDONLY|os.O_CREATE, 0666)
15-
}
16-
17-
func openFileReadWrite(filename string) (*os.File, error) {
18-
return os.OpenFile(filename, os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666)
19-
}
20-
218
func closeFileDeferred(f *os.File) {
229
defer f.Close() // NOT OK, if `f` is writable
2310
}
@@ -40,41 +27,48 @@ func closeFileDeferredIndirectReturn(f *os.File) {
4027
}
4128

4229
func deferredCalls() {
43-
if f, err := openFileWrite("foo.txt"); err != nil {
30+
// open file for writing
31+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
4432
closeFileDeferred(f) // NOT OK
4533
closeFileDeferredIndirect(f) // NOT OK
4634
}
4735

48-
if f, err := openFileRead("foo.txt"); err != nil {
36+
// open file for reading
37+
if f, err := os.OpenFile("foo.txt", os.O_RDONLY|os.O_CREATE, 0666); err != nil {
4938
closeFileDeferred(f) // OK
5039
closeFileDeferredIndirect(f) // OK
5140
}
5241

53-
if f, err := openFileReadWrite("foo.txt"); err != nil {
42+
// open file for reading and writing
43+
if f, err := os.OpenFile("foo.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
5444
closeFileDeferred(f) // NOT OK
5545
closeFileDeferredIndirect(f) // NOT OK
5646
}
5747
}
5848

5949
func notDeferred() {
60-
if f, err := openFileWrite("foo.txt"); err != nil {
50+
// open file for writing
51+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
6152
// the handle is write-only and we don't check if `Close` succeeds
6253
f.Close() // NOT OK
6354
}
6455

65-
if f, err := openFileRead("foo.txt"); err != nil {
56+
// open file for reading
57+
if f, err := os.OpenFile("foo.txt", os.O_RDONLY|os.O_CREATE, 0666); err != nil {
6658
// the handle is read-only, so this is ok
6759
f.Close() // OK
6860
}
6961

70-
if f, err := openFileReadWrite("foo.txt"); err != nil {
62+
// open file for reading and writing
63+
if f, err := os.OpenFile("foo.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
7164
// the handle is read-write and we don't check if `Close` succeeds
7265
f.Close() // NOT OK
7366
}
7467
}
7568

7669
func foo() error {
77-
if f, err := openFileWrite("foo.txt"); err != nil {
70+
// open file for writing
71+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
7872
// the result of the call to `Close` is returned to the caller
7973
return f.Close() // OK
8074
}
@@ -83,7 +77,8 @@ func foo() error {
8377
}
8478

8579
func isSyncedFirst() {
86-
if f, err := openFileWrite("foo.txt"); err != nil {
80+
// open file for writing
81+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
8782
// we have a call to `Sync` and check whether it was successful before proceeding
8883
if err := f.Sync(); err != nil {
8984
f.Close() // OK
@@ -93,7 +88,8 @@ func isSyncedFirst() {
9388
}
9489

9590
func deferredCloseWithSync() {
96-
if f, err := openFileWrite("foo.txt"); err != nil {
91+
// open file for writing
92+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
9793
// a call to `Close` is deferred, but we have a call to `Sync` later which
9894
// precedes the call to `Close` during execution
9995
defer f.Close() // OK
@@ -105,7 +101,8 @@ func deferredCloseWithSync() {
105101
}
106102

107103
func deferredCloseWithSyncEarlyReturn(n int) {
108-
if f, err := openFileWrite("foo.txt"); err != nil {
104+
// open file for writing
105+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
109106
// a call to `Close` is deferred
110107
defer f.Close() // NOT OK
111108

@@ -121,7 +118,8 @@ func deferredCloseWithSyncEarlyReturn(n int) {
121118
}
122119

123120
func unhandledSync() {
124-
if f, err := openFileWrite("foo.txt"); err != nil {
121+
// open file for writing
122+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
125123
// we have a call to `Sync` which precedes the call to `Close`, but there is no check
126124
// to see if `Sync` may have failed
127125
f.Sync()

0 commit comments

Comments
 (0)