libcdio-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Libcdio-devel] New git branch for bug 45017 ? Or re-use branch for


From: Thomas Schmitt
Subject: Re: [Libcdio-devel] New git branch for bug 45017 ? Or re-use branch for 45015 ?
Date: Mon, 31 Jul 2017 09:02:01 +0200

Hi,

i am not so happy with the comment change you made for
iso9660_check_dir_block_end().

Rocky Bernstein changed:
> - [...] The caller should
> - then skip the next actions in the loop and rather hop to the loop start
> - by "continue".
> - If "false" is returned, then processing of the caller's loop shall go on
> - normally.
> + [...] The caller often skips actions only when at the end of a
> + record list.

There is no "often" reaction but "always" the loop continuation with all
five callers. The prescription to do the "continue" is taken directly
from the five occasions where the new function calls replaced the original
code.
It is hard to imagine that this function is called outside a loop that
scans for directory records. So i thought it was necessary to talk
about the loop controlling aspect.

Given the live rythm of this software, we need to place an unambiguous
explanation for the programmers who look at this 10 years in the future.
(Let's hope we are all still around then and can play Stadler and Waldorf.)

So if my comment is not suitable for this, the replacement should still
tell the reader that "true" means
  "This is not a directory record. Re-loop, e.g. by continue, to get the
   next real directory record or to see the end of the directory."
and that "false" means
  "Go on with what you want to do with this directory record."


Minor concern:

> - Alleged directory record spans over block limit.
> + Directory record spans over block limit.

It is not a directory record. It's just a few bytes which the code
inspects whether they could be a directory record.
If its potential length indicating byte says that the potential record
spans over the block limit, then it is not such a record, but rather
some rogue bytes which all should be 0. Thus "alleged record".


> I still don't understand what "below does not exactly round up." means.

If the deviation of the necessary action from the arithmetical operation
of rounding is not a stumble stone for the reader of the code, then we
should simply trash that comment.
I thought it was necessary because i spent a few minutes pondering
over that special case.
But if it causes unnecessary pondering by the reader, then it is no good.


Have a nice day :)

Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]