From b7febb06afeb6e396f02ef1ee0acbdda93d3b57f Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 9 Jan 2019 18:40:17 +0000
Subject: [PATCH 1/5] CPP: Autoformat some Power of 10 queries.

---
 cpp/ql/src/Power of 10/Rule 1/UseOfJmp.ql       |  7 +++++--
 .../src/Power of 10/Rule 2/ExitPermanentLoop.ql | 12 +++++++-----
 cpp/ql/src/Power of 10/Rule 7/CheckArguments.ql | 17 +++++++++--------
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/cpp/ql/src/Power of 10/Rule 1/UseOfJmp.ql b/cpp/ql/src/Power of 10/Rule 1/UseOfJmp.ql
index 18b739f792ba..8eae5d2035d9 100644
--- a/cpp/ql/src/Power of 10/Rule 1/UseOfJmp.ql	
+++ b/cpp/ql/src/Power of 10/Rule 1/UseOfJmp.ql	
@@ -11,8 +11,11 @@ import cpp
 class ForbiddenFunction extends Function {
   ForbiddenFunction() {
     exists(string name | name = this.getName() |
-        name = "setjmp" or name = "longjmp" or
-        name = "sigsetjmp" or name = "siglongjmp")
+      name = "setjmp" or
+      name = "longjmp" or
+      name = "sigsetjmp" or
+      name = "siglongjmp"
+    )
   }
 }
 
diff --git a/cpp/ql/src/Power of 10/Rule 2/ExitPermanentLoop.ql b/cpp/ql/src/Power of 10/Rule 2/ExitPermanentLoop.ql
index ebb8a67d51ee..900581c7b83b 100644
--- a/cpp/ql/src/Power of 10/Rule 2/ExitPermanentLoop.ql	
+++ b/cpp/ql/src/Power of 10/Rule 2/ExitPermanentLoop.ql	
@@ -10,13 +10,15 @@ import cpp
 
 Stmt exitFrom(Loop l) {
   l.getAChild+() = result and
-  (result instanceof ReturnStmt or
-    exists(BreakStmt break | break = result |
-      not l.getAChild*() = break.getTarget())
+  (
+    result instanceof ReturnStmt
+    or
+    exists(BreakStmt break | break = result | not l.getAChild*() = break.getTarget())
   )
 }
 
 from Loop l, Stmt exit
-where l.getControllingExpr().getValue().toInt() != 0 and
-      exit = exitFrom(l)
+where
+  l.getControllingExpr().getValue().toInt() != 0 and
+  exit = exitFrom(l)
 select exit, "$@ should not be exited.", l, "This permanent loop"
diff --git a/cpp/ql/src/Power of 10/Rule 7/CheckArguments.ql b/cpp/ql/src/Power of 10/Rule 7/CheckArguments.ql
index e5d0a4d4d085..c6b1adb1ef0f 100644
--- a/cpp/ql/src/Power of 10/Rule 7/CheckArguments.ql	
+++ b/cpp/ql/src/Power of 10/Rule 7/CheckArguments.ql	
@@ -9,16 +9,17 @@
 import cpp
 
 predicate flow(Parameter p, ControlFlowNode n) {
-  (exists(p.getAnAccess()) and n = p.getFunction().getBlock()) or
-  exists(ControlFlowNode mid | flow(p, mid) and not mid = p.getAnAccess() and n = mid.getASuccessor())
+  (exists(p.getAnAccess()) and n = p.getFunction().getBlock())
+  or
+  exists(ControlFlowNode mid |
+    flow(p, mid) and not mid = p.getAnAccess() and n = mid.getASuccessor()
+  )
 }
 
-VariableAccess firstAccess(Parameter p) {
-  flow(p, result) and result = p.getAnAccess()
-}
+VariableAccess firstAccess(Parameter p) { flow(p, result) and result = p.getAnAccess() }
 
 from Parameter p, VariableAccess va
-where va = firstAccess(p) and
-    not exists(Expr e | e.isCondition() | e.getAChild*() = va)
+where
+  va = firstAccess(p) and
+  not exists(Expr e | e.isCondition() | e.getAChild*() = va)
 select va, "This use of parameter " + p.getName() + " has not been checked."
-

From 346bc1ac62a790a12450c08763b1b44ef0b10620 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 9 Jan 2019 18:43:19 +0000
Subject: [PATCH 2/5] CPP: Autoformat some code from Critical.

---
 cpp/ql/src/Critical/DeadCodeCondition.ql | 63 ++++++++++++++----------
 cpp/ql/src/Critical/NotInitialised.ql    | 25 ++++++----
 cpp/ql/src/Critical/SizeCheck.ql         | 53 +++++++++++---------
 cpp/ql/src/Critical/SizeCheck2.ql        | 56 +++++++++++----------
 4 files changed, 113 insertions(+), 84 deletions(-)

diff --git a/cpp/ql/src/Critical/DeadCodeCondition.ql b/cpp/ql/src/Critical/DeadCodeCondition.ql
index e7e5b4ef36cc..2f97ef14990b 100644
--- a/cpp/ql/src/Critical/DeadCodeCondition.ql
+++ b/cpp/ql/src/Critical/DeadCodeCondition.ql
@@ -7,51 +7,64 @@
  * @tags reliability
  *       external/cwe/cwe-561
  */
+
 import cpp
 
-predicate testAndBranch(Expr e, Stmt branch)
-{
-  exists(IfStmt ifstmt | ifstmt.getCondition() = e and
-    (ifstmt.getThen() = branch or ifstmt.getElse() = branch))
+predicate testAndBranch(Expr e, Stmt branch) {
+  exists(IfStmt ifstmt |
+    ifstmt.getCondition() = e and
+    (ifstmt.getThen() = branch or ifstmt.getElse() = branch)
+  )
   or
-  exists(WhileStmt while | while.getCondition() = e and
-    while.getStmt() = branch)
+  exists(WhileStmt while |
+    while.getCondition() = e and
+    while.getStmt() = branch
+  )
 }
 
-predicate choice(LocalScopeVariable v, Stmt branch, string value)
-{
+predicate choice(LocalScopeVariable v, Stmt branch, string value) {
   exists(AnalysedExpr e |
     testAndBranch(e, branch) and
     (
       (e.getNullSuccessor(v) = branch and value = "null")
       or
       (e.getNonNullSuccessor(v) = branch and value = "non-null")
-    ))
+    )
+  )
 }
 
-
-predicate guarded(LocalScopeVariable v, Stmt loopstart, AnalysedExpr child)
-{
+predicate guarded(LocalScopeVariable v, Stmt loopstart, AnalysedExpr child) {
   choice(v, loopstart, _) and
   loopstart.getChildStmt*() = child.getEnclosingStmt() and
   (definition(v, child) or exists(child.getNullSuccessor(v)))
 }
 
-predicate addressLeak(Variable v, Stmt leak)
-{
+predicate addressLeak(Variable v, Stmt leak) {
   exists(VariableAccess access |
     v.getAnAccess() = access and
     access.getEnclosingStmt() = leak and
-    access.isAddressOfAccess())
+    access.isAddressOfAccess()
+  )
 }
 
-from LocalScopeVariable v, Stmt branch, AnalysedExpr cond, string context, string test, string testresult
-where choice(v, branch, context)
-  and forall(ControlFlowNode def | definition(v, def) and definitionReaches(def, cond) | not guarded(v, branch, def))
-  and not cond.isDef(v)
-  and guarded(v, branch, cond)
-  and exists(cond.getNullSuccessor(v))
-  and not addressLeak(v, branch.getChildStmt*())
-  and ((cond.isNullCheck(v) and test = "null") or (cond.isValidCheck(v) and test = "non-null"))
-  and (if context = test then testresult = "succeed" else testresult = "fail")
-select cond, "Variable '" + v.getName() + "' is always " + context + " here, this check will always " + testresult + "."
+from
+  LocalScopeVariable v, Stmt branch, AnalysedExpr cond, string context, string test,
+  string testresult
+where
+  choice(v, branch, context) and
+  forall(ControlFlowNode def | definition(v, def) and definitionReaches(def, cond) |
+    not guarded(v, branch, def)
+  ) and
+  not cond.isDef(v) and
+  guarded(v, branch, cond) and
+  exists(cond.getNullSuccessor(v)) and
+  not addressLeak(v, branch.getChildStmt*()) and
+  (
+    (cond.isNullCheck(v) and test = "null")
+    or
+    (cond.isValidCheck(v) and test = "non-null")
+  ) and
+  (if context = test then testresult = "succeed" else testresult = "fail")
+select cond,
+  "Variable '" + v.getName() + "' is always " + context + " here, this check will always " +
+    testresult + "."
diff --git a/cpp/ql/src/Critical/NotInitialised.ql b/cpp/ql/src/Critical/NotInitialised.ql
index 4a336935c450..6236bf160a0d 100644
--- a/cpp/ql/src/Critical/NotInitialised.ql
+++ b/cpp/ql/src/Critical/NotInitialised.ql
@@ -7,10 +7,10 @@
  * @tags reliability
  *       external/cwe/cwe-457
  */
+
 import cpp
 
 // See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
-
 // Holds if s defines variable v (conservative)
 predicate defines(ControlFlowNode s, Variable lv) {
   exists(VariableAccess va | va = s and va.getTarget() = lv and va.isUsedAsLValue())
@@ -18,20 +18,23 @@ predicate defines(ControlFlowNode s, Variable lv) {
 
 // Holds if s uses variable v (conservative)
 predicate uses(ControlFlowNode s, Variable lv) {
-  exists(VariableAccess va | va = s and va.getTarget() = lv and va.isRValue()
-    and not va.getParent+() instanceof SizeofOperator)
+  exists(VariableAccess va |
+    va = s and
+    va.getTarget() = lv and
+    va.isRValue() and
+    not va.getParent+() instanceof SizeofOperator
+  )
 }
 
 // Holds if there is a path from the declaration of lv to n such that lv is
 // definitely not defined before n
 predicate noDefPath(LocalVariable lv, ControlFlowNode n) {
-     n.(DeclStmt).getADeclaration() = lv and not exists(lv.getInitializer())
-  or exists(ControlFlowNode p | noDefPath(lv, p) and n = p.getASuccessor() and not defines(p, lv))
+  n.(DeclStmt).getADeclaration() = lv and not exists(lv.getInitializer())
+  or
+  exists(ControlFlowNode p | noDefPath(lv, p) and n = p.getASuccessor() and not defines(p, lv))
 }
 
-predicate isAggregateType(Type t) {
-  t instanceof Class or t instanceof ArrayType
-}
+predicate isAggregateType(Type t) { t instanceof Class or t instanceof ArrayType }
 
 // Holds if va is a use of a local variable that has not been previously
 // defined
@@ -43,7 +46,8 @@ predicate undefinedLocalUse(VariableAccess va) {
     not lv.getType().hasName("va_list") and
     va = lv.getAnAccess() and
     noDefPath(lv, va) and
-    uses(va, lv))
+    uses(va, lv)
+  )
 }
 
 // Holds if gv is a potentially uninitialized global variable
@@ -53,7 +57,8 @@ predicate uninitialisedGlobal(GlobalVariable gv) {
     va = gv.getAnAccess() and
     va.isRValue() and
     not gv.hasInitializer() and
-    not gv.hasSpecifier("extern"))
+    not gv.hasSpecifier("extern")
+  )
 }
 
 from Element elt
diff --git a/cpp/ql/src/Critical/SizeCheck.ql b/cpp/ql/src/Critical/SizeCheck.ql
index efbd39897b79..83b8b385dbb8 100644
--- a/cpp/ql/src/Critical/SizeCheck.ql
+++ b/cpp/ql/src/Critical/SizeCheck.ql
@@ -11,56 +11,61 @@
  *       external/cwe/cwe-131
  *       external/cwe/cwe-122
  */
+
 import cpp
 
-class Allocation extends FunctionCall
-{
+class Allocation extends FunctionCall {
   Allocation() {
     exists(string name |
       this.getTarget().hasQualifiedName(name) and
-      (name = "malloc" or name = "calloc" or name = "realloc"))
+      (name = "malloc" or name = "calloc" or name = "realloc")
+    )
   }
 
   string getName() { result = this.getTarget().getQualifiedName() }
 
   int getSize() {
-      (this.getName() = "malloc" and
-      this.getArgument(0).getValue().toInt() = result)
+    (
+      this.getName() = "malloc" and
+      this.getArgument(0).getValue().toInt() = result
+    )
     or
-      (this.getName() = "realloc" and
-      this.getArgument(1).getValue().toInt() = result)
+    (
+      this.getName() = "realloc" and
+      this.getArgument(1).getValue().toInt() = result
+    )
     or
-      (this.getName() = "calloc" and
-      result =
-        this.getArgument(0).getValue().toInt() *
-        this.getArgument(1).getValue().toInt())
+    (
+      this.getName() = "calloc" and
+      result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
+    )
   }
 }
 
-predicate baseType(Allocation alloc, Type base)
-{
+predicate baseType(Allocation alloc, Type base) {
   exists(PointerType pointer |
     pointer.getBaseType() = base and
     (
       exists(AssignExpr assign |
-        assign.getRValue() = alloc and assign.getLValue().getType() = pointer)
+        assign.getRValue() = alloc and assign.getLValue().getType() = pointer
+      )
       or
-      exists(Variable v |
-        v.getInitializer().getExpr() = alloc and v.getType() = pointer)
+      exists(Variable v | v.getInitializer().getExpr() = alloc and v.getType() = pointer)
     )
   )
 }
 
-predicate decideOnSize(Type t, int size)
-{
+predicate decideOnSize(Type t, int size) {
   // If the codebase has more than one type with the same name, it can have more than one size.
   size = min(t.getSize())
 }
 
 from Allocation alloc, Type base, int basesize, int allocated
-where baseType(alloc, base)
-  and allocated = alloc.getSize()
-  and decideOnSize(base, basesize)
-  and basesize > allocated
-select alloc, "Type '" + base.getName() + "' is " + basesize.toString() +
-  " bytes, but only " + allocated.toString() + " bytes are allocated."
+where
+  baseType(alloc, base) and
+  allocated = alloc.getSize() and
+  decideOnSize(base, basesize) and
+  basesize > allocated
+select alloc,
+  "Type '" + base.getName() + "' is " + basesize.toString() + " bytes, but only " +
+    allocated.toString() + " bytes are allocated."
diff --git a/cpp/ql/src/Critical/SizeCheck2.ql b/cpp/ql/src/Critical/SizeCheck2.ql
index 273fd6f204a0..274887ce6d17 100644
--- a/cpp/ql/src/Critical/SizeCheck2.ql
+++ b/cpp/ql/src/Critical/SizeCheck2.ql
@@ -11,54 +11,60 @@
  *       external/cwe/cwe-131
  *       external/cwe/cwe-122
  */
+
 import cpp
 
-class Allocation extends FunctionCall
-{
+class Allocation extends FunctionCall {
   Allocation() {
     exists(string name |
       this.getTarget().hasQualifiedName(name) and
-      (name = "malloc" or name = "calloc" or name = "realloc"))
+      (name = "malloc" or name = "calloc" or name = "realloc")
+    )
   }
 
   string getName() { result = this.getTarget().getQualifiedName() }
 
   int getSize() {
-      (this.getName() = "malloc" and
-      this.getArgument(0).getValue().toInt() = result)
+    (
+      this.getName() = "malloc" and
+      this.getArgument(0).getValue().toInt() = result
+    )
     or
-      (this.getName() = "realloc" and
-      this.getArgument(1).getValue().toInt() = result)
+    (
+      this.getName() = "realloc" and
+      this.getArgument(1).getValue().toInt() = result
+    )
     or
-      (this.getName() = "calloc" and
-      result =
-        this.getArgument(0).getValue().toInt() *
-        this.getArgument(1).getValue().toInt())
+    (
+      this.getName() = "calloc" and
+      result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
+    )
   }
 }
 
-predicate baseType(Allocation alloc, Type base)
-{
+predicate baseType(Allocation alloc, Type base) {
   exists(PointerType pointer |
     pointer.getBaseType() = base and
     (
       exists(AssignExpr assign |
-        assign.getRValue() = alloc and assign.getLValue().getType() = pointer)
+        assign.getRValue() = alloc and assign.getLValue().getType() = pointer
+      )
       or
-      exists(Variable v |
-        v.getInitializer().getExpr() = alloc and v.getType() = pointer)
+      exists(Variable v | v.getInitializer().getExpr() = alloc and v.getType() = pointer)
     )
   )
 }
 
 from Allocation alloc, Type base, int basesize, int allocated
-where baseType(alloc, base)
-  and allocated = alloc.getSize()
+where
+  baseType(alloc, base) and
+  allocated = alloc.getSize() and
   // If the codebase has more than one type with the same name, check if any matches
-  and not exists(int size | base.getSize() = size |
-    size = 0
-    or (allocated / size) * size = allocated)
-  and basesize = min(base.getSize())
-select alloc, "Allocated memory (" + allocated.toString() +
-  " bytes) is not a multiple of the size of '" +
-  base.getName() + "' (" + basesize.toString() + " bytes)."
+  not exists(int size | base.getSize() = size |
+    size = 0 or
+    (allocated / size) * size = allocated
+  ) and
+  basesize = min(base.getSize())
+select alloc,
+  "Allocated memory (" + allocated.toString() + " bytes) is not a multiple of the size of '" +
+    base.getName() + "' (" + basesize.toString() + " bytes)."

From c4b01d08168b998146518e5bd0e55e49a1d39f1c Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 10 Jan 2019 15:06:51 +0000
Subject: [PATCH 3/5] CPP: Autoformat some other untidy source files.

---
 .../Cleanup-DuplicateIncludeGuard.ql          | 17 +++-
 .../Likely Bugs/Format/SnprintfOverflow.ql    | 92 ++++++++++---------
 .../LogicalExprCouldBeSimplified.ql           | 36 +++++---
 3 files changed, 80 insertions(+), 65 deletions(-)

diff --git a/cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard.ql b/cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard.ql
index 9e4f2d1276dd..c06cfeabe0d0 100644
--- a/cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard.ql	
+++ b/cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard.ql	
@@ -9,6 +9,7 @@
  *       maintainability
  *       modularity
  */
+
 import cpp
 import semmle.code.cpp.headers.MultipleInclusion
 
@@ -20,9 +21,15 @@ import semmle.code.cpp.headers.MultipleInclusion
  * However one case must be a correctIncludeGuard to prove that this macro really is intended
  * to be an include guard.
  */
+
 from HeaderFile hf, PreprocessorDirective ifndef, string macroName, int num
-where hasIncludeGuard(hf, ifndef, _, macroName)
-and exists(HeaderFile other | hasIncludeGuard(other, _, _, macroName) and hf.getShortName() != other.getShortName())
-and num = strictcount(HeaderFile other | hasIncludeGuard(other, _, _, macroName))
-and correctIncludeGuard(_, _, _, _, macroName)
-select ifndef, "The macro name '" + macroName + "' of this include guard is used in " + num + " different header files."
+where
+  hasIncludeGuard(hf, ifndef, _, macroName) and
+  exists(HeaderFile other |
+    hasIncludeGuard(other, _, _, macroName) and hf.getShortName() != other.getShortName()
+  ) and
+  num = strictcount(HeaderFile other | hasIncludeGuard(other, _, _, macroName)) and
+  correctIncludeGuard(_, _, _, _, macroName)
+select ifndef,
+  "The macro name '" + macroName + "' of this include guard is used in " + num +
+    " different header files."
diff --git a/cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql b/cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql
index 6d0d29ad335a..c5e933fc200c 100644
--- a/cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql	
+++ b/cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql	
@@ -1,7 +1,6 @@
 /**
  * @name Potentially overflowing call to snprintf
  * @description Using the return value from snprintf without proper checks can cause overflow.
- *
  * @kind problem
  * @problem.severity warning
  * @precision high
@@ -20,44 +19,44 @@ import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
  * true if there is an arithmetic operation on the path that
  * might overflow.
  */
-predicate flowsToExpr(
-  Expr source, Expr sink, boolean pathMightOverflow) {
+predicate flowsToExpr(Expr source, Expr sink, boolean pathMightOverflow) {
   // Might the current expression overflow?
-  exists (boolean otherMightOverflow
-  | flowsToExprImpl(source, sink, otherMightOverflow)
-  | if convertedExprMightOverflow(sink)
-      then pathMightOverflow = true
-      else pathMightOverflow = otherMightOverflow)
+  exists(boolean otherMightOverflow | flowsToExprImpl(source, sink, otherMightOverflow) |
+    if convertedExprMightOverflow(sink)
+    then pathMightOverflow = true
+    else pathMightOverflow = otherMightOverflow
+  )
 }
 
 /**
  * Implementation of `flowsToExpr`. Does everything except
  * checking whether the current expression might overflow.
  */
-predicate flowsToExprImpl(
-  Expr source, Expr sink, boolean pathMightOverflow) {
-  (source = sink and pathMightOverflow = false and
-   source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength())
+predicate flowsToExprImpl(Expr source, Expr sink, boolean pathMightOverflow) {
+  (
+    source = sink and
+    pathMightOverflow = false and
+    source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength()
+  )
   or
-  exists (RangeSsaDefinition def, LocalScopeVariable v
-  | flowsToDef(source, def, v, pathMightOverflow) and
-    sink = def.getAUse(v))
+  exists(RangeSsaDefinition def, LocalScopeVariable v |
+    flowsToDef(source, def, v, pathMightOverflow) and
+    sink = def.getAUse(v)
+  )
   or
-  flowsToExpr(
-    source, sink.(UnaryArithmeticOperation).getOperand(), pathMightOverflow)
+  flowsToExpr(source, sink.(UnaryArithmeticOperation).getOperand(), pathMightOverflow)
   or
-  flowsToExpr(
-    source, sink.(BinaryArithmeticOperation).getAnOperand(), pathMightOverflow)
+  flowsToExpr(source, sink.(BinaryArithmeticOperation).getAnOperand(), pathMightOverflow)
   or
-  flowsToExpr(
-    source, sink.(Assignment).getRValue(), pathMightOverflow)
+  flowsToExpr(source, sink.(Assignment).getRValue(), pathMightOverflow)
   or
-  flowsToExpr(
-    source, sink.(AssignOperation).getLValue(), pathMightOverflow)
+  flowsToExpr(source, sink.(AssignOperation).getLValue(), pathMightOverflow)
   or
-  exists (FormattingFunctionCall call
-  | sink = call and
-    flowsToExpr(source, call.getArgument(call.getTarget().getSizeParameterIndex()), pathMightOverflow))
+  exists(FormattingFunctionCall call |
+    sink = call and
+    flowsToExpr(source, call.getArgument(call.getTarget().getSizeParameterIndex()),
+      pathMightOverflow)
+  )
 }
 
 /**
@@ -67,14 +66,14 @@ predicate flowsToExprImpl(
  * on the path that might overflow.
  */
 predicate flowsToDef(
-  Expr source, RangeSsaDefinition def, LocalScopeVariable v,
-  boolean pathMightOverflow) {
+  Expr source, RangeSsaDefinition def, LocalScopeVariable v, boolean pathMightOverflow
+) {
   // Might the current definition overflow?
-  exists (boolean otherMightOverflow
-  | flowsToDefImpl(source, def, v, otherMightOverflow)
-  | if defMightOverflow(def, v)
-      then pathMightOverflow = true
-      else pathMightOverflow = otherMightOverflow)
+  exists(boolean otherMightOverflow | flowsToDefImpl(source, def, v, otherMightOverflow) |
+    if defMightOverflow(def, v)
+    then pathMightOverflow = true
+    else pathMightOverflow = otherMightOverflow
+  )
 }
 
 /**
@@ -89,26 +88,29 @@ predicate flowsToDef(
  * the path. But it is a good way to reduce the number of false positives.
  */
 predicate flowsToDefImpl(
-  Expr source, RangeSsaDefinition def, LocalScopeVariable v,
-  boolean pathMightOverflow) {
+  Expr source, RangeSsaDefinition def, LocalScopeVariable v, boolean pathMightOverflow
+) {
   // Assignment or initialization: `e = v;`
-  exists (Expr e
-  | e = def.getDefiningValue(v) and
-    flowsToExpr(source, e, pathMightOverflow))
+  exists(Expr e |
+    e = def.getDefiningValue(v) and
+    flowsToExpr(source, e, pathMightOverflow)
+  )
   or
   // `x++`
-  exists (CrementOperation crem
-  | def = crem and
+  exists(CrementOperation crem |
+    def = crem and
     crem.getOperand() = v.getAnAccess() and
-    flowsToExpr(source, crem.getOperand(), pathMightOverflow))
+    flowsToExpr(source, crem.getOperand(), pathMightOverflow)
+  )
   or
   // Phi definition.
   flowsToDef(source, def.getAPhiInput(v), v, pathMightOverflow)
 }
 
 from FormattingFunctionCall call, Expr sink
-where flowsToExpr(call, sink, true)
-and sink = call.getArgument(call.getTarget().getSizeParameterIndex())
-select
-  call, "The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.",
+where
+  flowsToExpr(call, sink, true) and
+  sink = call.getArgument(call.getTarget().getSizeParameterIndex())
+select call,
+  "The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.",
   sink, "size argument"
diff --git a/cpp/ql/src/Likely Bugs/Likely Typos/LogicalExprCouldBeSimplified.ql b/cpp/ql/src/Likely Bugs/Likely Typos/LogicalExprCouldBeSimplified.ql
index 2d0596428210..9847b0c4a711 100644
--- a/cpp/ql/src/Likely Bugs/Likely Typos/LogicalExprCouldBeSimplified.ql	
+++ b/cpp/ql/src/Likely Bugs/Likely Typos/LogicalExprCouldBeSimplified.ql	
@@ -8,6 +8,7 @@
  * @problem.severity warning
  * @tags maintainability
  */
+
 import cpp
 
 /**
@@ -15,15 +16,15 @@ import cpp
  * or template argument).
  */
 predicate simple(Literal l) {
-    l instanceof OctalLiteral or
-    l instanceof HexLiteral or
-    l instanceof CharLiteral or
-    l.getValueText() = "true" or
-    l.getValueText() = "false" or
-    // Parsing doubles is too slow...
-    //exists(l.getValueText().toFloat())
-    // Instead, check whether the literal starts with a letter.
-    not l.getValueText().regexpMatch("[a-zA-Z_].*")
+  l instanceof OctalLiteral or
+  l instanceof HexLiteral or
+  l instanceof CharLiteral or
+  l.getValueText() = "true" or
+  l.getValueText() = "false" or
+  // Parsing doubles is too slow...
+  //exists(l.getValueText().toFloat())
+  // Instead, check whether the literal starts with a letter.
+  not l.getValueText().regexpMatch("[a-zA-Z_].*")
 }
 
 predicate booleanLiteral(Literal l) {
@@ -32,18 +33,23 @@ predicate booleanLiteral(Literal l) {
 }
 
 string boolLiteralInLogicalOp(Literal literal) {
-   booleanLiteral(literal) and literal.getParent() instanceof BinaryLogicalOperation and
-  result = "Literal value " + literal.getValueText() + " is used in a logical expression; simplify or use a constant."
+  booleanLiteral(literal) and
+  literal.getParent() instanceof BinaryLogicalOperation and
+  result = "Literal value " + literal.getValueText() +
+      " is used in a logical expression; simplify or use a constant."
 }
 
 string comparisonOnLiterals(ComparisonOperation op) {
-  simple(op.getLeftOperand()) and simple(op.getRightOperand()) and
+  simple(op.getLeftOperand()) and
+  simple(op.getRightOperand()) and
   not op.getAnOperand().isInMacroExpansion() and
-  if op.isConstant() then result = "This comparison involves two literals and is always " + op.getValue() + "."
+  if op.isConstant()
+  then result = "This comparison involves two literals and is always " + op.getValue() + "."
   else result = "This comparison involves two literals and should be simplified."
 }
 
 from Expr e, string msg
-where (msg = boolLiteralInLogicalOp(e) or msg = comparisonOnLiterals(e)) and
-    not e.isInMacroExpansion()
+where
+  (msg = boolLiteralInLogicalOp(e) or msg = comparisonOnLiterals(e)) and
+  not e.isInMacroExpansion()
 select e, msg

From ba3bc1596b24cba534a11cab177a2bab343e39d7 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 9 Jan 2019 18:45:33 +0000
Subject: [PATCH 4/5] CPP: Manual fixup.

---
 cpp/ql/src/Critical/NotInitialised.ql | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpp/ql/src/Critical/NotInitialised.ql b/cpp/ql/src/Critical/NotInitialised.ql
index 6236bf160a0d..f7ad3caf01e4 100644
--- a/cpp/ql/src/Critical/NotInitialised.ql
+++ b/cpp/ql/src/Critical/NotInitialised.ql
@@ -11,6 +11,7 @@
 import cpp
 
 // See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
+
 // Holds if s defines variable v (conservative)
 predicate defines(ControlFlowNode s, Variable lv) {
   exists(VariableAccess va | va = s and va.getTarget() = lv and va.isUsedAsLValue())

From 87569d14b97fd694ebbdcceb6c09ce11ae86ffa8 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 10 Jan 2019 17:38:42 +0000
Subject: [PATCH 5/5] CPP: QLDoc comments.

---
 cpp/ql/src/Critical/NotInitialised.ql | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/cpp/ql/src/Critical/NotInitialised.ql b/cpp/ql/src/Critical/NotInitialised.ql
index f7ad3caf01e4..a69abbbb3f12 100644
--- a/cpp/ql/src/Critical/NotInitialised.ql
+++ b/cpp/ql/src/Critical/NotInitialised.ql
@@ -12,12 +12,16 @@ import cpp
 
 // See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
 
-// Holds if s defines variable v (conservative)
+/**
+ * Holds if `s` defines variable `v` (conservative).
+ */
 predicate defines(ControlFlowNode s, Variable lv) {
   exists(VariableAccess va | va = s and va.getTarget() = lv and va.isUsedAsLValue())
 }
 
-// Holds if s uses variable v (conservative)
+/**
+ * Holds if `s` uses variable `v` (conservative).
+ */
 predicate uses(ControlFlowNode s, Variable lv) {
   exists(VariableAccess va |
     va = s and
@@ -27,8 +31,10 @@ predicate uses(ControlFlowNode s, Variable lv) {
   )
 }
 
-// Holds if there is a path from the declaration of lv to n such that lv is
-// definitely not defined before n
+/**
+ * Holds if there is a path from the declaration of `lv` to `n` such that `lv` is
+ * definitely not defined before `n`.
+ */
 predicate noDefPath(LocalVariable lv, ControlFlowNode n) {
   n.(DeclStmt).getADeclaration() = lv and not exists(lv.getInitializer())
   or
@@ -37,8 +43,10 @@ predicate noDefPath(LocalVariable lv, ControlFlowNode n) {
 
 predicate isAggregateType(Type t) { t instanceof Class or t instanceof ArrayType }
 
-// Holds if va is a use of a local variable that has not been previously
-// defined
+/**
+ * Holds if `va` is a use of a local variable that has not been previously
+ * defined.
+ */
 predicate undefinedLocalUse(VariableAccess va) {
   exists(LocalVariable lv |
     // it is hard to tell when a struct or array has been initialized, so we
@@ -51,7 +59,9 @@ predicate undefinedLocalUse(VariableAccess va) {
   )
 }
 
-// Holds if gv is a potentially uninitialized global variable
+/**
+ * Holds if `gv` is a potentially uninitialized global variable.
+ */
 predicate uninitialisedGlobal(GlobalVariable gv) {
   exists(VariableAccess va |
     not isAggregateType(gv.getUnderlyingType()) and