[Python-Dev] [Python-checkins] cpython: Touch up exception messaging

[Nick Coghlan]

On Sun, Jan 27, 2013 at 3:25 AM, Ezio Melotti <ezio.melotti at gmail.com> wrote:
> Hi,
> I'm not sure these changes are an improvement.
>
> On Fri, Jan 25, 2013 at 8:49 PM, brett.cannon
> <python-checkins at python.org> wrote:
>> http://hg.python.org/cpython/rev/792810303239
>> changeset:   81735:792810303239
>> user:        Brett Cannon <brett at python.org>
>> date:        Fri Jan 25 13:49:19 2013 -0500
>> summary:
>>   Touch up exception messaging
>>      [...]
>>      magic = data[:4]
>>      raw_timestamp = data[4:8]
>>      raw_size = data[8:12]
>>      if magic != _MAGIC_BYTES:
>> -        msg = 'bad magic number in {!r}: {!r}'.format(name, magic)
>> +        msg = 'incomplete magic number in {!r}: {!r}'.format(name, magic)
>
> Here 2 things could go wrong:
>  1) magic is less than 4 bytes (so it could be "incomplete");
>  2) magic is 4 bytes, but it's different (so it's "invalid" or "bad");
> For this to be "incomplete" the size of the whole file should be less
> than 4 bytes, and it's unlikely that in a 3bytes-long file the content
> is an incomplete magic number.  It's much more likely than it's not a
> magic number at all, or that it is a bad/wrong/invalid 4-bytes magic
> number, so the previous error looked better to me.

Oops, I missed that - this one wasn't supposed to change. It was only
the other two that were misleading.

>
>>          raise ImportError(msg, **exc_details)
>>      elif len(raw_timestamp) != 4:
>> -        message = 'bad timestamp in {!r}'.format(name)
>> +        message = 'incomplete timestamp in {!r}'.format(name)
>>          _verbose_message(message)
>>          raise EOFError(message)
>>      elif len(raw_size) != 4:
>> -        message = 'bad size in {!r}'.format(name)
>> +        message = 'incomplete size in {!r}'.format(name)
>
> If we arrived here the magic number was right, so this are probably
> the timestamp and size, but for this to fail the whole file should be
> less than 8 or 12 bytes (unless I misread the code).
> Something like "reached EOF while reading timestamp/size" would
> probably be more informative.

I don't mind one way or the other here. "bad" was wrong, since it
implied this code actually checked these values against the expected
ones, but it doesn't, it merely requires that they exist. "incomplete
<value>" or "reached EOF while reading <value>" are both correct and a
sufficient hint for anyone already familiar with how the bytecode
cache format works.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia