Re: Credential improvements need review

[Junio C Hamano <gitster@pobox.com>]

M Hickford <mirth.hickford@gmail.com> writes:

>> * mh/credential-erase-improvements-more (2023-06-24) 2 commits
>>  - credential/wincred: erase matching creds only
>>  - credential/libsecret: erase matching creds only
>> 
>>  Needs review.
>>  source: <pull.1529.git.git.1687596777147.gitgitgadget@gmail.com>
>
> Hi. Is anyone able to help review these changes?
>
> https://lore.kernel.org/git/pull.1529.git.git.1687596777147.gitgitgadget@gmail.com/
> https://lore.kernel.org/git/pull.1527.git.git.1687591293705.gitgitgadget@gmail.com/

Thanks for pinging.  One thing that may help (both patches, my
understanding is that they are of the same spirit, just one is for
libsecret while the other one is for wincred) is to describe the
problem the patches attempt to address a bit more.  For example,
in one of them:

    Fix test "helper ... does not erase a password distinct from input"
    introduced in aeb21ce22e (credential: avoid erasing distinct password,
    2023-06-13)

we can read from the above proposed log message that it is a "fix"
to some bug, and that the "bug" was introduced by the named commit,
but there are a few things that it does not explain, that could have
helped readers to convince themselves that the changes in the patches
are addressing the right problems and solving them in the right
way.  For example,

 * how does the "bug" manifest itself in an observable way to the
   end-users?  "When they do X, they expect Y to happen, but instead
   Z happens, and doing Z breaks expectation of users expecting Y in
   this (W) way."

 * what was wrong in the code that led to the "bug"?  Was it testing
   a wrong condition before making a call to some system service?
   Was the condition it checked correct but it made a call in a
   wrong way (and if so how)?

Thanks.