ldandersen/scifihifi-iphone

Problem with deleteItemForUsername in SFHFKeychainUtils.m

arielitovsky opened this issue · 2 comments

The line (423 on
if (error != nil && status != noErr)

Should be
if (error != nil || status != noErr)

Because it is possible for the error to be nil but for the status to return something different. In that case (e.g. the item was not found) we would want to return NO

That would be incorrect as well:
When error != nil, your clause evaluates to YES regardless of the value of status. Hence the method returns NO — even if the operation succeeded!
In fact, it's even harmful:
When error == nil and status != noErr, your clause evaluates to YES, i.e. *error means *nil, which crashes with a segfault.

So in essence, status != noErr is necessary and sufficient for the method to return NO, while error != nil is neither. It is, however, necessary for the assignment *error = ... to behave correctly.

As a side note, I totally don't understand why but although no errors are reported, the item isn't deleted from the keychain :( On 10.8.2 (Mac) — any idea? Someone mind testing?