code review 222080043: taowm: fix nil dereferences when xgb functions return a... (issue 222080043 by nigeltao@golang.org)

[nige...@golang.org]

Reviewers: nigeltao,

Message:
Hello nige...@golang.org (cc: ta...@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/taowm/


Description:
taowm: fix nil dereferences when xgb functions return a nil result and
a nil error.

Please review this at https://codereview.appspot.com/222080043/

Affected files (+43, -27 lines):
M taowm/actions.go
M taowm/geom.go
M taowm/main.go
M taowm/xinit.go


Index: taowm/actions.go
===================================================================
--- a/taowm/actions.go
+++ b/taowm/actions.go
@@ -215,8 +215,11 @@
}
k1.layout()
k0.layout()
- if p, err := xp.QueryPointer(xConn, rootXWin).Reply(); err != nil {
+ p, err := xp.QueryPointer(xConn, rootXWin).Reply()
+ if err != nil {
log.Println(err)
+ }
+ if p == nil {
k1.focusFrame(k1.mainFrame.firstDescendent())
} else {
k1.focusFrame(k1.frameContaining(p.RootX, p.RootY))
@@ -408,7 +411,7 @@
k.fullscreen = !k.fullscreen
if p, err := xp.QueryPointer(xConn, rootXWin).Reply(); err != nil {
log.Println(err)
- } else {
+ } else if p != nil {
k.focusFrame(k.frameContaining(p.RootX, p.RootY))
}
k.configure()
@@ -500,8 +503,11 @@
}

func finishMergeSplit(k *workspace) {
- if p, err := xp.QueryPointer(xConn, rootXWin).Reply(); err != nil {
+ p, err := xp.QueryPointer(xConn, rootXWin).Reply()
+ if err != nil {
log.Println(err)
+ }
+ if p == nil {
k.focusFrame(k.mainFrame.firstDescendent())
} else {
k.focusFrame(k.frameContaining(p.RootX, p.RootY))
Index: taowm/geom.go
===================================================================
--- a/taowm/geom.go
+++ b/taowm/geom.go
@@ -158,7 +158,7 @@
if len(k.list) != 0 {
if p, err := xp.QueryPointer(xConn, rootXWin).Reply(); err != nil {
log.Println(err)
- } else {
+ } else if p != nil {
k.index = k.indexForPoint(p.RootX, p.RootY)
}
}
@@ -430,7 +430,6 @@
return f.lastDescendent()
}
}
- panic("unreachable")
}

func (f *frame) drawBorder() {
@@ -445,6 +444,9 @@
if err != nil {
log.Println(err)
}
+ if p == nil {
+ return ""
+ }
return string(p.Value)
}

Index: taowm/main.go
===================================================================
--- a/taowm/main.go
+++ b/taowm/main.go
@@ -107,7 +107,7 @@
if prop, err := xp.GetProperty(xConn, false, xWin, atomWMProtocols,
xp.GetPropertyTypeAny, 0, 64).Reply(); err != nil {
log.Println(err)
- } else {
+ } else if prop != nil {
for v := prop.Value; len(v) >= 4; v = v[4:] {
switch xp.Atom(u32(v)) {
case atomWMDeleteWindow:
@@ -122,17 +122,19 @@
if prop, err := xp.GetProperty(xConn, false, xWin, atomWMTransientFor,
xp.GetPropertyTypeAny, 0, 64).Reply(); err != nil {
log.Println(err)
- } else if v := prop.Value; len(v) == 4 {
- transientForXWin := xp.Window(u32(v))
- transientFor = findWindow(func(w *window) bool {
- return w.xWin == transientForXWin
- })
+ } else if prop != nil {
+ if v := prop.Value; len(v) == 4 {
+ transientForXWin := xp.Window(u32(v))
+ transientFor = findWindow(func(w *window) bool {
+ return w.xWin == transientForXWin
+ })
+ }
}

k := screens[0].workspace
if p, err := xp.QueryPointer(xConn, rootXWin).Reply(); err != nil {
log.Println(err)
- } else {
+ } else if p != nil {
k = screenContaining(p.RootX, p.RootY).workspace
}
w = &window{
@@ -237,7 +239,7 @@
replacement.configure()
if p, err := xp.QueryPointer(xConn, rootXWin).Reply(); err != nil {
log.Println(err)
- } else if contains(f.rect, p.RootX, p.RootY) {
+ } else if p != nil && contains(f.rect, p.RootX, p.RootY) {
focus(replacement)
}
} else {
@@ -281,22 +283,22 @@
initScreens()

// Manage any existing windows.
- tree, err := xp.QueryTree(xConn, rootXWin).Reply()
- if err != nil {
+ if tree, err := xp.QueryTree(xConn, rootXWin).Reply(); err != nil {
log.Fatal(err)
- }
- for _, c := range tree.Children {
- if c == desktopXWin {
- continue
+ } else if tree != nil {
+ for _, c := range tree.Children {
+ if c == desktopXWin {
+ continue
+ }
+ attrs, err := xp.GetWindowAttributes(xConn, c).Reply()
+ if attrs == nil || err != nil {
+ continue
+ }
+ if attrs.OverrideRedirect || attrs.MapState == xp.MapStateUnmapped {
+ continue
+ }
+ manage(c, false)
}
- attrs, err := xp.GetWindowAttributes(xConn, c).Reply()
- if err != nil {
- continue
- }
- if attrs.OverrideRedirect || attrs.MapState == xp.MapStateUnmapped {
- continue
- }
- manage(c, false)
}

// Process X events.
Index: taowm/xinit.go
===================================================================
--- a/taowm/xinit.go
+++ b/taowm/xinit.go
@@ -53,6 +53,9 @@
if err != nil {
log.Fatal(err)
}
+ if r == nil {
+ return 0
+ }
return r.Atom
}

@@ -157,6 +160,9 @@
if err != nil {
log.Fatal(err)
}
+ if km == nil {
+ log.Fatal("couldn't get keyboard mapping")
+ }
n := int(km.KeysymsPerKeycode)
if n < 2 {
log.Fatalf("too few keysyms per keycode: %d", n)

[nige...@golang.org]

LGTM.

https://codereview.appspot.com/222080043/

[nige...@golang.org]

*** Submitted as
https://code.google.com/p/taowm/source/detail?r=84cda96db559 ***

taowm: fix nil dereferences when xgb functions return a nil result and
a nil error.

TBR=nigeltao
R=nigeltao
CC=taowm
https://codereview.appspot.com/222080043


https://codereview.appspot.com/222080043/