hi seth,
thanks for the patchset!
On Wed, Apr 01, 2026 at 02:44:24PM +1000, Seth McDonald wrote:
[PATCH 1/4] Remove excessive if nesting
Such control flow is not the most readable, particularly for longer
functions, due to the added indentation.
that's true, but irrelevant for short functions. this was violated a bit
in ensure_password due to the later addition of the macos code, but
you're rectifying this in the next patch. this really makes this patch
counter-productive churn imo. please drop it.
fwiw, isync's indentation is tab width independent, but it's visually
optimized for four spaces. with this setting, somewhat deeper indents
aren't a problem.
On Wed, Apr 01, 2026 at 02:44:26PM +1000, Seth McDonald wrote:
[PATCH 3/4] Remove use of deprecated SecKeychain API
isync's macOS keychain functionality interfaces with Apple's Security
framework via the SecKeychain API. The SecItem API was introduced in
macOS 10.6 and became the preferred method using the keychain on macOS,
officially deprecating the SecKeychain API in macOS 10.10. So the
codebase is currently using an API deprecated since 2014.
"was using"
unlike many other projects, i write commit messages such that they
actually match the idea of describing a transition, so there is no
ambiguous "current" state that relies on convention.
This can be seen by building isync on macOS. Running `make` on macOS
26.4 with gcc 15.2.0 gives relevant compilation warnings: [...]
you can drop that; it doesn't add any value imo.
The SecItem API requires the creation of multiple objects to specify
the
search query, each of which may fail for whatever reason (most likely
OOM).
For now, use assertions to ensure no such object creation fails.
yeah, that's just ugly.
just swap the two patches.
search_macos_keychain( const char *host, const char *user )
{
- void *password_data;
+ CFDictionaryRef query;
please localize the declarations more consistently.
+ CFDataRef password_data;
i think i'd call this just "password" ...
+ const char *password_buffer;
.. and leave that as password_data. less churn, and it retains the
symmetry with _length.
char *password_copy;
- UInt32 password_length;
+ CFIndex password_length;
OSStatus ret;
- ret = SecKeychainFindInternetPassword(
+ /* Same index = key-value pair */
prefer c99 style for inline comments.
(traditional style is ok for function docs.)
_______________________________________________
isync-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/isync-devel
