On Wed, Apr 19, 2023 at 08:30:35AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > Here's the patch that I came up with, though it does not distinguish > > between "we did not see any trust level" and "gpg told us the trust > > level was undefined". I think that's OK. That level is still below > > TRUST_NEVER. But if we really want to distinguish we can introduce a new > > value for the enum. > > Good. > > In my zeroth draft, I added to the enum a new TRUST_FAILED = -1 to > be used for the initialization assignment and get stringified in the > gpg_trust_level_to_str() function, which gave us the distinction and > made sure the enum is signed. But in the end, I decided it was not > worth risking upsetting the end-user scripts that assumed the > current set of levels with a new "level" that is not known to them. > > Initializing to undefined like this patch is with much less damage > to the codebase, and existing end-user scripts are probably prepared > to react to "undefined" already and treat it as even less trustworthy > than the "never" ones. One thing that I wondered about for using UNDEFINED is that we do this: static enum signature_trust_level configured_min_trust_level = TRUST_UNDEFINED; which is then later compared with: status |= sigc->result != 'G'; status |= sigc->trust_level < configured_min_trust_level; So before my patch the uninitialized state is (supposedly) less than the min level, and after they are the same. For the reasons I gave in the commit message, I think that less-than comparison was already broken. And likewise, for the reasons I gave, it hopefully never matters since the result would never be 'G' in that case. So I think it's fine, but I definitely had to stare at it for a while. This all comes from 54887b4689 (gpg-interface: add minTrustLevel as a configuration option, 2019-12-27), which does discuss some of the implications, but I think my patch is in line with the logic there. -Peff
