himitsu: Use new for-each loops where it makes sense v1 APPLIED

Hugo Osvaldo Barrera: 1
 Use new for-each loops where it makes sense

 13 files changed, 70 insertions(+), 116 deletions(-)

[PATCH himitsu] Use new for-each loops where it makes sense Export this patch

The only externally visible change is that 'secstore::next' now returns
'done' when iteration is done, whereas previously it returned 'void'
---
 cmd/himitsu-store/main.ha | 26 ++++++++------------
 cmd/himitsud/cmd.ha       | 16 ++----------
 cmd/himitsud/main.ha      |  3 +--
 cmd/hiprompt-tty/main.ha  |  8 +++---
 cmd/hiq/main.ha           | 51 ++++++++++++++++++---------------------
 config/conf.ha            |  9 +------
 himitsu/query/parse.ha    | 22 ++++++++---------
 prompt/prompter.ha        |  5 +---
 secstore/compat.ha        |  4 +--
 secstore/delete.ha        |  7 +++---
 secstore/query.ha         | 14 +++++------
 secstore/secstore.ha      | 18 ++++----------
 secstore/types.ha         |  3 +--
 13 files changed, 70 insertions(+), 116 deletions(-)

diff --git a/cmd/himitsu-store/main.ha b/cmd/himitsu-store/main.ha
index a4a23ae..9d3e761 100644
--- a/cmd/himitsu-store/main.ha
+++ b/cmd/himitsu-store/main.ha
@@ -34,8 +34,7 @@ export fn main() void = {
 	let re = false;
 	let initstore = false;
 	let chpw = false;
-	for (let i = 0z; i < len(cmd.opts); i += 1) {
-		const opt = cmd.opts[i];
+	for (let opt .. cmd.opts) {
 		switch (opt.0) {
 		case 'r' =>
 			re = true;
@@ -285,22 +284,17 @@ export fn reencrypt_move(oldpass: []u8, newpass: []u8) (void | secstore::error)
 	fmt::error("Copying all entries to the new store... ")!;
 	const q = query::query { ... };
 	const iter = secstore::query(&oldstore, &q, false);
-	for (true) {
-		const item = match (secstore::next(&oldstore, &iter)) {
-		case let item: *secstore::entry =>
-			let buf = memio::dynamic();
-			defer io::close(&buf)!;
-			secstore::write(&oldstore, &buf, item, true)!;
+	for (let item => secstore::next(&oldstore, &iter)) {
+		let buf = memio::dynamic();
+		defer io::close(&buf)!;
+		secstore::write(&oldstore, &buf, item, true)!;
 
-			io::seek(&buf, 0, io::whence::SET)!;
+		io::seek(&buf, 0, io::whence::SET)!;
 
-			let iq = query::parse(&buf)!;
-			defer query::finish(&iq);
-			secstore::add(&store, &iq)!;
-			yield item;
-		case void =>
-			break;
-		};
+		let iq = query::parse(&buf)!;
+		defer query::finish(&iq);
+		secstore::add(&store, &iq)!;
+		yield item;
 	};
 
 	secstore::close(&oldstore);
diff --git a/cmd/himitsud/cmd.ha b/cmd/himitsud/cmd.ha
index c6d8dc7..239d8a2 100644
--- a/cmd/himitsud/cmd.ha
+++ b/cmd/himitsud/cmd.ha
@@ -143,13 +143,7 @@ fn exec_del(serv: *server, client: *client, args: []str) (void | cmderror) = {
 
 	const iter = secstore::query(serv.store, &q, strict);
 	let matches: []*secstore::entry = [];
-	for (true) {
-		const item = match (secstore::next(serv.store, &iter)) {
-		case let item: *secstore::entry =>
-			yield item;
-		case void =>
-			break;
-		};
+	for (let item => secstore::next(serv.store, &iter)) {
 		fmt::fprint(&buf, "key ")?;
 		secstore::write(serv.store, &buf, item, false)?;
 		io::write(&buf, ['\n'])?;
@@ -222,13 +216,7 @@ fn exec_query(serv: *server, client: *client, args: []str) (void | cmderror) = {
 
 	const iter = secstore::query(serv.store, &q, strict);
 	let matches: []*secstore::entry = [];
-	for (true) {
-		const item = match (secstore::next(serv.store, &iter)) {
-		case let item: *secstore::entry =>
-			yield item;
-		case void =>
-			break;
-		};
+	for (let item => secstore::next(serv.store, &iter)) {
 		append(matches, item);
 	};
 
diff --git a/cmd/himitsud/main.ha b/cmd/himitsud/main.ha
index 607a724..2548512 100644
--- a/cmd/himitsud/main.ha
+++ b/cmd/himitsud/main.ha
@@ -23,8 +23,7 @@ export fn main() void = {
 	defer getopt::finish(&cmd);
 
 	let daemonize = false;
-	for (let i = 0z; i < len(cmd.opts); i += 1) {
-		const opt = &cmd.opts[i];
+	for (let opt &.. cmd.opts) {
 		switch (opt.0) {
 		case 'D' =>
 			daemonize = true;
diff --git a/cmd/hiprompt-tty/main.ha b/cmd/hiprompt-tty/main.ha
index fd3a74e..433beb9 100644
--- a/cmd/hiprompt-tty/main.ha
+++ b/cmd/hiprompt-tty/main.ha
@@ -40,8 +40,8 @@ export fn main() void = {
 		...
 	};
 	defer {
-		for (let i = 0z; i < len(ctx.keys); i += 1) {
-			query::finish(&ctx.keys[i]);
+		for (let key &.. ctx.keys) {
+			query::finish(key);
 		};
 		free(ctx.keys);
 		io::close(ctx.tty)!;
@@ -123,9 +123,9 @@ fn prompt(ctx: *context) void = {
 	fmt::fprintfln(ctx.tty,
 		"An application has requested {} of the following keys:",
 		mode)!;
-	for (let i = 0z; i < len(ctx.keys); i += 1) {
+	for (let key &.. ctx.keys) {
 		fmt::fprintf(ctx.tty, "\t")!;
-		query::unparse(ctx.tty, &ctx.keys[i])!;
+		query::unparse(ctx.tty, key)!;
 	};
 
 	fmt::fprint(ctx.tty, "Proceed? [y/N] ")!;
diff --git a/cmd/hiq/main.ha b/cmd/hiq/main.ha
index 0e42d44..7774557 100644
--- a/cmd/hiq/main.ha
+++ b/cmd/hiq/main.ha
@@ -34,8 +34,8 @@ export fn main() void = {
 	let flags: client::flags = 0;
 	let field: (str | void) = void;
 	let one = false;
-	for (let i = 0z; i < len(cmd.opts); i += 1) {
-		switch (cmd.opts[i].0) {
+	for (let opt .. cmd.opts) {
+		switch (opt.0) {
 		case '1' =>
 			one = true;
 		case 'a' =>
@@ -45,7 +45,7 @@ export fn main() void = {
 		case 'D' =>
 			op = client::operation::DEL;
 		case 'F' =>
-			field = cmd.opts[i].1;
+			field = opt.1;
 		case 'Q' =>
 			op = client::operation::QUIT;
 		case 's' =>
@@ -76,31 +76,27 @@ export fn main() void = {
 		if (tty::isatty(os::stderr_file) && tty::isatty(os::stdin_file)) {
 			fmt::errorln("Enter new keys, one per line, then press ^d:")!;
 		};
-		for (true) {
-			match (bufio::read_line(os::stdin)!) {
-			case io::EOF => break;
-			case let line: []u8 =>
-				// NB. Can't defer free(line), causes a
-				// use-after-free in fmt::fatal
-				const line = match (strings::fromutf8(line)) {
-				case let s: str =>
-					yield s;
-				case =>
-					free(line);
-					fmt::fatal("Error: Query is not valid UTF-8");
-				};
-				const query = match (query::parse_str(line)) {
-				case let q: query::query =>
-					yield q;
-				case query::invalid =>
-					fmt::fatal("Invalid query:", line);
-				case query::dupkeys =>
-					fmt::fatal("Duplicate keys in query:", line);
-				};
-				defer query::finish(&query);
+		for (let line => bufio::read_line(os::stdin)!) {
+			// NB. Can't defer free(line), causes a
+			// use-after-free in fmt::fatal
+			const line = match (strings::fromutf8(line)) {
+			case let s: str =>
+				yield s;
+			case =>
 				free(line);
-				send(conn, client::operation::ADD, &query, flags, field, one);
+				fmt::fatal("Error: Query is not valid UTF-8");
 			};
+			const query = match (query::parse_str(line)) {
+			case let q: query::query =>
+				yield q;
+			case query::invalid =>
+				fmt::fatal("Invalid query:", line);
+			case query::dupkeys =>
+				fmt::fatal("Duplicate keys in query:", line);
+			};
+			defer query::finish(&query);
+			free(line);
+			send(conn, client::operation::ADD, &query, flags, field, one);
 		};
 	} else {
 		const query = match (query::parse_items(cmd.args)) {
@@ -145,8 +141,7 @@ fn send(
 		};
 		match (field) {
 		case let field: str =>
-			for (let i = 0z; i < len(key.items); i += 1) {
-				let item = key.items[i];
+			for (let item &.. key.items) {
 				if (item.key == field) {
 					fmt::fprintln(&buf, item.value)!;
 				};
diff --git a/config/conf.ha b/config/conf.ha
index f8c9434..b7361ce 100644
--- a/config/conf.ha
+++ b/config/conf.ha
@@ -41,14 +41,7 @@ export fn load() (config | error) = {
 
 	const scanner = ini::scan(file);
 	defer ini::finish(&scanner);
-	for (true) {
-		const entry = match (ini::next(&scanner)?) {
-		case let entry: ini::entry =>
-			yield entry;
-		case io::EOF =>
-			break;
-		};
-
+	for (let entry => ini::next(&scanner)?) {
 		switch (entry.0) {
 		case "himitsud" =>
 			conf_himitsud(&conf, &entry);
diff --git a/himitsu/query/parse.ha b/himitsu/query/parse.ha
index e9c2722..76ef64a 100644
--- a/himitsu/query/parse.ha
+++ b/himitsu/query/parse.ha
@@ -89,8 +89,8 @@ export fn parse_items(items: []str) (query | error) = {
 	let keys: []str = [];
 	defer free(keys);
 
-	for (let i = 0z; i < len(items); i += 1) {
-		const (key, value) = strings::cut(items[i], "=");
+	for (let item .. items) {
+		const (key, value) = strings::cut(item, "=");
 		let optional = false, private = false;
 		if (strings::hassuffix(key, "!")) {
 			private = true;
@@ -135,9 +135,7 @@ export fn parse_items(items: []str) (query | error) = {
 // caller must pass the return value to [[finish]] after use.
 export fn dup_pub(q: *query) query = {
 	let query = query { ... };
-	for (let i = 0z; i < len(q.items); i += 1) {
-		let p = q.items[i];
-
+	for (let p .. q.items) {
 		p.key = strings::dup(p.key);
 		p.value = if (p.private) "" else strings::dup(p.value);
 
@@ -148,12 +146,12 @@ export fn dup_pub(q: *query) query = {
 
 // Frees resources associated with this query.
 export fn finish(q: *query) void = {
-	for (let i = 0z; i < len(q.items); i += 1) {
-		free(q.items[i].key);
-		if (q.items[i].private) {
-			bytes::zero(strings::toutf8(q.items[i].value));
+	for (let item .. q.items) {
+		free(item.key);
+		if (item.private) {
+			bytes::zero(strings::toutf8(item.value));
 		};
-		free(q.items[i].value);
+		free(item.value);
 	};
 	free(q.items);
 };
@@ -195,8 +193,8 @@ export fn finish(q: *query) void = {
 		`foo=bar foo!=baz`,
 		`foo=bar bar=bay foo=baz`,
 	];
-	for (let i = 0z; i < len(errcases); i += 1) {
-		const input = memio::fixed(strings::toutf8(errcases[i]));
+	for (let err .. errcases) {
+		const input = memio::fixed(strings::toutf8(err));
 		assert(parse(&input) is dupkeys);
 	};
 };
diff --git a/prompt/prompter.ha b/prompt/prompter.ha
index 9e3d76a..8854442 100644
--- a/prompt/prompter.ha
+++ b/prompt/prompter.ha
@@ -94,10 +94,7 @@ export fn wait_unlock(
 	prompt: *prompter,
 	store: *secstore::secstore,
 ) (bool | error) = {
-	for (true) match (bufio::read_line(prompt.stdout)?) {
-	case io::EOF =>
-		break;
-	case let buf: []u8 =>
+	for (let buf => bufio::read_line(prompt.stdout)?) {
 		defer {
 			bytes::zero(buf);
 			free(buf);
diff --git a/secstore/compat.ha b/secstore/compat.ha
index 2a4aa61..7850f46 100644
--- a/secstore/compat.ha
+++ b/secstore/compat.ha
@@ -36,8 +36,8 @@ fn compatquery_parse(in: io::handle) (query::query | query::error | io::error) =
 	let ok = false;
 	defer if (!ok) query::finish(&q);
 
-	for (let i = 0z; i < len(items); i += 1) {
-		const (key, value) = strings::cut(items[i], "=");
+	for (let item .. items) {
+		const (key, value) = strings::cut(item, "=");
 		let optional = false, private = false;
 		if (strings::hassuffix(key, "!")) {
 			private = true;
diff --git a/secstore/delete.ha b/secstore/delete.ha
index aa0c933..275ea21 100644
--- a/secstore/delete.ha
+++ b/secstore/delete.ha
@@ -33,8 +33,8 @@ export fn del(store: *secstore, q: *query::query) (void | locked) = {
 	path::push(&buf, "..", "index.1")!;
 
 	const index = os::create(path::string(&buf), 0o600)!;
-	for (let i = 0z; i < len(store.entries); i += 1) {
-		add_index(store, &store.entries[i]);
+	for (let entry &.. store.entries) {
+		add_index(store, entry);
 	};
 
 	store.index = index;
@@ -44,8 +44,7 @@ export fn del(store: *secstore, q: *query::query) (void | locked) = {
 fn delkeys(store: *secstore, entry: *entry) void = {
 	let buf = path::init()!;
 
-	for (let i = 0z; i < len(entry.pairs); i += 1) {
-		const pair = &entry.pairs[i];
+	for (let pair &.. entry.pairs) {
 		const id = match (pair.value) {
 		case let id: uuid::uuid =>
 			yield id;
diff --git a/secstore/query.ha b/secstore/query.ha
index 7ead686..089571f 100644
--- a/secstore/query.ha
+++ b/secstore/query.ha
@@ -24,8 +24,8 @@ export fn query(
 	};
 };
 
-// Returns the next matching item from an iterator, or void if none remain.
-export fn next(store: *secstore, iter: *iterator) (*entry | void) = {
+// Returns the next matching item from an iterator, or done if none remain.
+export fn next(store: *secstore, iter: *iterator) (*entry | done) = {
 	for (iter.index < len(store.entries); iter.index += 1) {
 		const ent = &store.entries[iter.index];
 		if (entry_match(ent, iter.query, iter.strict)) {
@@ -33,12 +33,12 @@ export fn next(store: *secstore, iter: *iterator) (*entry | void) = {
 			return ent;
 		};
 	};
+	return done;
 };
 
 fn entry_match(ent: *entry, query: *query::query, strict: bool) bool = {
 	let nmatched = 0z;
-	for (let i = 0z; i < len(query.items); i += 1) {
-		const q = &query.items[i];
+	for (let q &.. query.items) {
 		const p = match (findpair(ent, q.key)) {
 		case void =>
 			if (!q.optional) {
@@ -69,9 +69,9 @@ fn entry_match(ent: *entry, query: *query::query, strict: bool) bool = {
 };
 
 fn findpair(ent: *entry, key: str) (*pair | void) = {
-	for (let i = 0z; i < len(ent.pairs); i += 1) {
-		if (ent.pairs[i].key == key) {
-			return &ent.pairs[i];
+	for (let pair &.. ent.pairs) {
+		if (pair.key == key) {
+			return pair;
 		};
 	};
 };
diff --git a/secstore/secstore.ha b/secstore/secstore.ha
index 1612611..f093525 100644
--- a/secstore/secstore.ha
+++ b/secstore/secstore.ha
@@ -328,8 +328,8 @@ export fn close(store: *secstore) void = {
 };
 
 fn free_keys(store: *secstore) void = {
-	for (let i = 0z; i < len(store.entries); i += 1) {
-		entry_finish(&store.entries[i]);
+	for (let entry .. store.entries) {
+		entry_finish(&entry);
 	};
 };
 
@@ -350,8 +350,7 @@ export fn add(store: *secstore, q: *query::query) (*entry | locked | dupentry) =
 
 	// TODO: Better error handling
 	let pairs: []pair = [];
-	for (let i = 0z; i < len(q.items); i += 1) {
-		const item = q.items[i];
+	for (let item .. q.items) {
 		assert(item.value != "", "Invalid query submitted to secstore::add");
 		const val = if (item.private) {
 			yield add_secret(store, item.value);
@@ -434,13 +433,7 @@ fn load_index(store: *secstore) (void | io::error | errors::invalid) = {
 	defer bytes::zero(key);
 	keystore::read(store.key as keystore::key, key);
 
-	for (true) {
-		const line = match (bufio::read_line(store.index)?) {
-		case let line: []u8 =>
-			yield line;
-		case io::EOF =>
-			break;
-		};
+	for (let line => bufio::read_line(store.index)?) {
 		defer free(line);
 
 		const buf = memio::fixed(line);
@@ -461,8 +454,7 @@ fn load_index(store: *secstore) (void | io::error | errors::invalid) = {
 		defer query::finish(&q);
 
 		let pairs: []pair = [];
-		for (let i = 0z; i < len(q.items); i += 1) {
-			const item = q.items[i];
+		for (let item .. q.items) {
 			const val = if (item.private) {
 				yield uuid::decodestr(item.value)!;
 			} else {
diff --git a/secstore/types.ha b/secstore/types.ha
index eae73ed..8964b1d 100644
--- a/secstore/types.ha
+++ b/secstore/types.ha
@@ -35,8 +35,7 @@ export type pair = struct {
 };
 
 fn entry_finish(ent: *entry) void = {
-	for (let i = 0z; i < len(ent.pairs); i += 1) {
-		const pair = &ent.pairs[i];
+	for (let pair &.. ent.pairs) {
 		free(pair.key);
 		match (pair.value) {
 		case let val: str =>
-- 
2.44.0

Armin Preiml <apreiml@strohwolke.at>

11 months ago

Thanks! Applied with a minor fix.

To https://git.sr.ht/~sircmpwn/himitsu
   c657d70..6becc84  master -> master