On Thu, May 26, 2016 at 04:18:32PM +0200, Fabiano Fidêncio wrote:
Please, see the attached patches.
Hey Fabiano,
Thank you for the patches!
I admit I haven't tested the patches yet, just scrolled through the
diffs. See some comments inline. But I would also like someone else to
chime in because we talked about the code on IRC, so I'm not the right
person to review the patches on my own.
Best Regards,
--
Fabiano Fidêncio
From 6985f8c5ac2491829af99d71b2ecb7926a38fe26 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com>
Date: Wed, 25 May 2016 20:16:58 +0200
Subject: [PATCH 1/4] sysdb: move add_string() convenience to sysdb.c
OK, makes sense to do isolated changes.
From fbb071581f0b90ffe85499d46641038e8b55ccaa Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com>
Date: Wed, 25 May 2016 21:12:18 +0200
Subject: [PATCH 2/4] sysdb: add sysdb_{add,replace,delete}_string()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As the add_string() convenience can add, replace or delete a string
according to the operation received as its argument, some confusion can
easily happen due to its misleading name.
In order to improve the explicitness of our code, let's introduce
sysdb_add_string(), sysdb_replace_string() and sysdb_delete_string().
These new functions are basically wrappers of add_string() (now
sysdb_ldb_msg_string_helper()), calling it using the proper flag
according to each function.
Any code previously using add_string() is now adapted to use these brand
new functions.
I just wonder about the names here. The new functions operate on
ldb_message, so I guess a better name might be something like
sysdb_ldb_msg*.
The reason is that the 'native' type of sysdb is sysdb_attrs and without
the function name saying otherwise, I would expect the function to operate
on sysdb_attrs.
From f4e3d4e3e7fb51c3a61fe2014217a930ae3a0091 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com>
Date: Wed, 25 May 2016 21:40:28 +0200
Subject: [PATCH 3/4] sysdb: move add_ulong() convenience to sysdb.c
Makes sense to move code before changes in an isolated change.
From daaf63d4e378611a2fbba541dafe9dedae7934eb Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com>
Date: Wed, 25 May 2016 23:00:59 +0200
Subject: [PATCH 4/4] sysdb: add sysdb_{add,replace,delete}_ulong()
Same comment about the name here as I had about patch 2/4.