"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
some nits for diff.git
To:
gameoftrees@openbsd.org
Date:
Tue, 02 Aug 2022 20:09:09 +0200

Download raw body.

Thread
spotted while reading the code / playing with the tests.  in order:

 - make diff_atom_hash_update private to diff_atomize_text.c;  it's
   debatable but i think it doesn't belong to the public diff_main.h header

 - reuse diff_atom_hash_update instead of handrolling it.  no-op change
   but avoid issues if we ever want to change the hashing.

 - optionally look for diff in obj/

 - less chatty output: avoids some printing to stderr (like "gpatch not
   found" or "rm: errors: No such file or directory").  I'm also
   moving from which(1) to the command built-in.

ok?

diff refs/remotes/got/main refs/heads/main
commit - 405e89a6da28edc7555666affbad86a823e2c460
commit + 10e19102f24e3bd105eaf997c9fad8e49f6e7ad7
blob - 5e816ae10b5948c0aeacd27365a91f3147b96baf
blob + 8dba472a6b08ba2fd590485c9c5279ebeb98cf0a
--- include/diff_main.h
+++ include/diff_main.h
@@ -39,11 +39,6 @@ struct diff_atom {
 	unsigned int hash;
 };
 
-/* Mix another atom_byte into the provided hash value and return the result.
- * The hash value passed in for the first byte of the atom must be zero. */
-unsigned int
-diff_atom_hash_update(unsigned int hash, unsigned char atom_byte);
-
 /* Compare two atoms for equality. Return 0 on success, or errno on failure.
  * Set cmp to -1, 0, or 1, just like strcmp(). */
 int
blob - 79d9633bdaa73ce679a74c04294b35271c5374f7
blob + b7fc3e0f685bcde8614b4795d59fc479c7f4338c
--- lib/diff_atomize_text.c
+++ lib/diff_atomize_text.c
@@ -29,7 +29,11 @@
 #include "diff_internal.h"
 #include "diff_debug.h"
 
-unsigned int
+/*
+ * Mix another atom_byte into the provided hash value and return the result.
+ * The hash value passed in for the first byte of the atom must be zero.
+ */
+static unsigned int
 diff_atom_hash_update(unsigned int hash, unsigned char atom_byte)
 {
 	return hash * 23 + atom_byte;
@@ -143,7 +147,7 @@ diff_data_atomize_text_lines_mmap(struct diff_data *d)
 		while (line_end < end && *line_end != '\r' && *line_end != '\n') {
 			if (!ignore_whitespace
 			    || !isspace(*line_end))
-				hash = hash * 23 + *line_end;
+				hash = diff_atom_hash_update(hash, *line_end);
 			if (*line_end == '\0')
 				embedded_nul = true;
 			line_end++;
blob - d8d3aa95464f45b8a21779276ef6a7de72e4c09f
blob + 84f898a2d3fd7e827f37d2c2472e5d90bc945d84
--- test/verify_all.sh
+++ test/verify_all.sh
@@ -1,10 +1,13 @@
 #!/bin/sh
 
-diff_prog="../diff/diff"
+diff_prog="../diff/obj/diff"
+if [ ! -x $diff_prog ]; then
+	diff_prog="../diff/diff"
+fi
 
 # At present, test015 only passes with GNU patch.
 # Larry's patch has a bug with empty files in combination with -R...
-if which gpatch > /dev/null; then
+if command -v gpatch >/dev/null 2>&1; then
 	patch_prog="gpatch"
 else
 	patch_prog="patch"
@@ -12,7 +15,7 @@ fi
 
 diff_type=unidiff
 
-rm errors
+rm -f errors
 
 verify_diff_script() {
 	orig_left="$1"