Maxime Villard <max%M00nBSD.net@localhost> writes:
> Le 17/07/2014 06:01, matthew green a écrit :
>>>> + if (secsize < DEV_BSIZE) {
>>>> +#ifdef DIAGNOSTIC
>>>> + printf("Invalid block secsize %d\n", secsize);
>>>> +#endif
>>>> + error = EINVAL;
>>>> + goto error_exit;
>>>> + }
>>>>
>>>> if (argp->flags & MSDOSFSMNT_GEMDOSFS) {
>>>> if (secsize != GEMDOSFS_BSIZE) {
>>>
>>> That's an abuse of DIAGNOSTIC. It should only be used to control
>>> whether to assert on things that cannot happen. (Bad data on a mount
>>> point is not a software invariant.)
>>
>> agreed -- these sorts of messages should be under DEBUG.
>>
>
> I don't see what's wrong with that; on -current at least if it
> breaks something people will immediately get a clear message, so
> that if we someone says "I get invalid ..." we can quickly revert
> the change.
>
> I think my commit was clear on that:
>
> "Put the printf under DIAGNOSTIC temporarily to see if someone complains."
> ^^^^^^^^^^^
We are not objecting to the message. The rule for DIAGNOSTIC is that it
only adds asserts. So you are breaking that rule, and doing it for no
good reason. If you want to see if this hits and people see the mesage,
then make the printf unconditional, and put enough context in it so that
someone who isn't looking for this might figure it out.
Longer term, putting it under DEBUG makes sense, as users would be
flooded with messages for every.
So I really did mean to object to even a temporary misuses of
DIAGNOSTIC.
Attachment:
pgpV1GUej95ga.pgp
Description: PGP signature