Re: [PATCH] Re: Parent directory's permissions in mini-status

[Leonard den Ottolander <leonard den ottolander nl>]

Hi Pavel,

On Mon, 2005-03-28 at 18:42, Pavel Tsekov wrote:
> > On Sat, 2005-04-02 at 13:54, Leonard den Ottolander wrote:
> > > Just discussed this with Roland Illig who came up with a patch that
> > > seems to work correctly. I further fixed his patch to not need
> 
> Well, he should have done some testing too. Not just hacking.

It was my responsibility to post the patch. As there is no comment in
the code to explain why ".." is excluded we didn't quite get why this
was being done. Of course we could have investigated a bit more, but
then the answers also seem to come by posting this patch ;) .

> Please, don't commit neither to HEAD nor to PRE. This patch sucks.

>From the post you reference I somewhat get the issue. Found a changelog
entry as well that indicates stat()ing of ".." might take a long time on
certain file systems. If such a comment could be added to the code we
could avoid people making the same mistake as Roland and I did in the
future.

It's just that much of the code is so poorly documented that it is not
always obvious why certain solutions have been chosen.

>  It
> actually prevents the user from going to the parent directory for at least
> the following filesystems:
> 
>   FISH, cpio, tar

Aha. 

> I am seriously starting to worry about the quality of the code.

Which parts exactly? I put up this code for discussion, and it is quite
clear to me now why it should not be applied. As you know I always put
some effort in getting things discussed before they are being committed.

If your clock hadn't been off by a week I would have probably read your
reply to Sergey before trying to cook up a patch together with Roland
:-p .

Anyway, the objections are clear and I would propose to add a short
comment to the header of add_dotdot_to_list() explaining why we prefer
to use a bogus entry for "..".

Leonard.

-- 
mount -t life -o ro /dev/dna /genetic/research