Commit cd99d918bf for openssl.org
commit cd99d918bf26de00b2905c654968215f845fdd55
Author: Norbert Pocs <norbertp@openssl.org>
Date: Thu Oct 29 10:09:34 2026 +0000
REVIEW.md: describe how review comments are written
The reviewer's comments restate the same finding three times, lead
with an awkward actor-first sentence and cite bare file names and line
numbers. Ask for a maintainer's voice instead: the defect first, each
fact once, and full repository paths for every location.
Assisted-by: Claude Code:claude-opus-5-5
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
Merge-date: Wed Nov 11 05:33:59 2026
Merged-from: https://github.com/openssl/openssl/pull/33446
diff --git a/REVIEW.md b/REVIEW.md
index 1a3babe168..32abad5f9f 100644
--- a/REVIEW.md
+++ b/REVIEW.md
@@ -29,8 +29,8 @@ or message being parsed, or a file or network stream, and problems that
break a security property: memory safety, secrecy of keys and secrets,
authentication or verification results, constant-time behaviour, or
enforcement of mandatory properties such as `fips=yes`. The first
-sentence of an Important finding says who can trigger it and what they
-gain.
+sentence of an Important finding names the bug. The second says who can
+trigger it and what they gain.
Functional bugs, behaviour changes, performance regressions, misuse of a
public API by the calling application, and `STYLE.md` and
@@ -42,9 +42,9 @@ Evidence
--------
Each finding states the inputs or state that trigger it and what goes
-wrong, with a `file:line` citation for every claim about code outside the
-diff. Read the callee before claiming what it returns or who owns its
-result. A finding that cannot name a trigger is not reported.
+wrong, with a `path/from/root.c:line` citation for every claim about
+code outside the diff. Read the callee before claiming what it returns
+or who owns its result. A finding that cannot name a trigger is not reported.
Error paths and ownership
-------------------------
@@ -220,6 +220,21 @@ violates.
- Pointers are compared explicitly with `NULL`. Magic numbers are named
or explained.
+Writing the comment
+-------------------
+
+Write like a maintainer reviewing a colleague's patch. Start with what
+is wrong in this diff, address the author directly, and say each fact
+once. Give the trigger and the fix in plain sentences. No preamble, no
+restating, no closing summary. Do not relate a finding to other
+findings unless fixing one changes the other.
+
+Cite a location once, where it matters, always as the full path from
+the repository root with the line number, such as
+`crypto/evp/evp.c:4312`. For several lines in one file, give the path
+once: `crypto/evp/evp.c:4312, 4350, 4401` for separate lines,
+`crypto/evp/evp.c:4312-4330` for a block. Never use a bare file name.
+
Cap the nits
------------