From ebe7d65db55a09676d4435bc74b65d1f68603130 Mon Sep 17 00:00:00 2001
From: Mark Wooding <mdw@distorted.org.uk>
Date: Mon, 25 Nov 2019 12:00:52 +0000
Subject: [PATCH] key.c: Make the `KeyFile' key-lookup methods behave
 consistently.

Annoyingly, `byid' and `bytag' would raise `KERR_NOTFOUND', while
`bytype' would give you `None'.  Resolve this: add a `fail' keyword
argument to each method, which defaults to true: if `fail' is true then
a failed lookup raises `KERR_NOTFOUND'; if false, then you get `None'.
---
 key.c      | 48 +++++++++++++++++++++++++++++++++---------------
 t/t-key.py |  9 ++++++---
 2 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/key.c b/key.c
index 017c973..82bbb19 100644
--- a/key.c
+++ b/key.c
@@ -1718,40 +1718,58 @@ end:
   return (0);
 }
 
-static PyObject *kfmeth_byid(PyObject *me, PyObject *arg)
+static PyObject *kfmeth_byid(PyObject *me, PyObject *arg, PyObject *kw)
 {
   uint32 id;
   key *k;
+  PyObject *failp = Py_True;
   PyObject *rc = 0;
+  static const char *const kwlist[] = { "id", "fail", 0 };
 
-  if (!PyArg_ParseTuple(arg, "O&:byid", convu32, &id)) goto end;
-  if ((k = key_byid(KEYFILE_KF(me), id)) == 0) KEYERR(KERR_NOTFOUND);
-  rc = key_pywrap(me, k);
+  if (!PyArg_ParseTupleAndKeywords(arg, kw, "O&|O:byid", KWLIST,
+				   convu32, &id, &failp))
+    goto end;
+  if ((k = key_byid(KEYFILE_KF(me), id)) != 0) rc = key_pywrap(me, k);
+  else if (PyObject_IsTrue(failp)) KEYERR(KERR_NOTFOUND);
+  else RETURN_NONE;
 end:
   return (rc);
 }
 
-static PyObject *kfmeth_bytype(PyObject *me, PyObject *arg)
+static PyObject *kfmeth_bytype(PyObject *me, PyObject *arg, PyObject *kw)
 {
   char *type;
   key *k;
+  PyObject *failp = Py_True;
   PyObject *rc = 0;
+  static const char *const kwlist[] = { "type", "fail", 0 };
 
-  if (!PyArg_ParseTuple(arg, "s:bytype", &type)) goto end;
-  if ((k = key_bytype(KEYFILE_KF(me), type)) == 0) RETURN_NONE;
-  rc = key_pywrap(me, k);
+  if (!PyArg_ParseTupleAndKeywords(arg, kw, "s|O:bytype", KWLIST,
+				   &type, &failp))
+    goto end;
+  if ((k = key_bytype(KEYFILE_KF(me), type)) != 0) rc = key_pywrap(me, k);
+  else if (PyObject_IsTrue(failp)) KEYERR(KERR_NOTFOUND);
+  else RETURN_NONE;
 end:
   return (rc);
 }
 
-static PyObject *kfmeth_bytag(PyObject *me, PyObject *arg)
+static PyObject *kfmeth_bytag(PyObject *me, PyObject *arg, PyObject *kw)
 {
   PyObject *tagobj;
   key *k;
+  PyObject *failp = Py_True;
+  PyObject *rc = 0;
+  static const char *const kwlist[] = { "type", "fail", 0 };
 
-  if (!PyArg_ParseTuple(arg, "O:bytag", &tagobj)) return (0);
-  if ((k = bytag(me, tagobj)) == 0) RETURN_NONE;
-  else return (key_pywrap(me, k));
+  if (!PyArg_ParseTupleAndKeywords(arg, kw, "O|O:bytag", KWLIST,
+				   &tagobj, &failp))
+    goto end;
+  if ((k = bytag(me, tagobj)) != 0) rc = key_pywrap(me, k);
+  else if (PyObject_IsTrue(failp)) KEYERR(KERR_NOTFOUND);
+  else RETURN_NONE;
+end:
+  return (rc);
 }
 
 static PyObject *kfmeth_newkey(PyObject *me, PyObject *arg, PyObject *kw)
@@ -1820,9 +1838,9 @@ static const PyMethodDef keyfile_pymethods[] = {
 					   "[report = <built-in-reporter>])")
   KWMETH(newkey,	"KF.newkey(ID, TYPE, [exptime = KEXP_FOREVER]) "
 								    "-> KEY")
-  METH	(byid,		"KF.byid(KEYID) -> KEY|None")
-  METH	(bytype,	"KF.bytype(TYPE) -> KEY|None")
-  METH	(bytag,		"KF.bytag(TAG) -> KEY|None")
+  KWMETH(byid,		"KF.byid(KEYID, [fail = True]) -> KEY|None")
+  KWMETH(bytype,	"KF.bytype(TYPE, [fail = True]) -> KEY|None")
+  KWMETH(bytag,		"KF.bytag(TAG, [fail = True]) -> KEY|None")
   KWMETH(qtag,		"KF.qtag(TAG, [new = KD]) -> FULLTAG, KEY, OLDKD")
   GMAP_ROMETHODS
 #undef METHNAME
diff --git a/t/t-key.py b/t/t-key.py
index 000dc11..27808a4 100644
--- a/t/t-key.py
+++ b/t/t-key.py
@@ -163,11 +163,14 @@ class TestKeyFile (U.TestCase):
 
     ## Check unsuccessful searches.
     me.assertRaises(KeyError, lambda: kf["notexist"])
-    me.assertEqual(kf.bytag("notexist"), None)
-    me.assertEqual(kf.bytag(12345), None)
-    me.assertEqual(kf.bytype("notexist"), None)
+    me.assertRaises(C.KeyError, kf.bytag, "notexist")
+    me.assertRaises(C.KeyError, kf.bytag, 12345)
+    me.assertEqual(kf.bytag("notexist", fail = False), None)
+    me.assertRaises(C.KeyError, kf.bytype, "notexist")
     me.assertRaises(TypeError, kf.bytype, 12345)
+    me.assertEqual(kf.bytype("notexist", fail = False), None)
     me.assertRaises(C.KeyError, kf.byid, 0x12345678)
+    me.assertEqual(kf.byid(0x12345678, fail = False), None)
 
     me.assertRaises(C.KeyError, kf.mergeline, "nowhere", 2, "")
 
-- 
2.11.0