LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruen@suse.de>
To: Neil Brown <neilb@suse.de>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Andrew Morton <akpm@osdl.org>, Tony Jones <tonyj@suse.de>
Subject: Re: [PATCH] Fix d_path for lazy unmounts
Date: Wed, 14 Feb 2007 18:43:49 -0800	[thread overview]
Message-ID: <200702141843.49668.agruen@suse.de> (raw)
In-Reply-To: <17862.30489.694079.138765@notabene.brown>

On Sunday 04 February 2007 16:15, Neil Brown wrote:
> The behaviour in the face of a lazy unmount should be clarified in
> this comment.

Done.

> If sys_getcwd is called on a directory that is no longer
> connected to the root, it isn't clear to me that it should return
> without an error.
> Without your patch it can return garbage which is clearly wrong.
> With you patch it will return a relative path name, which is also
> wrong (it isn't a valid path that leads to the current working directory).

Right, it should return -ENOENT instead in that case. Fixed as well.

> I would suggest that 'fail_deleted' be (e.g.) changed to
> 'fail_condition' where two conditions are defined
> #define DPATH_FAIL_DELETED 1
> #define DPATH_FAIL_DISCONNECTED 2

The much cleaner interface is to check if the path returned starts with a 
slash. If it doesn't, we know the path is bad as far as sys_getcwd() is 
concerned. We will construct the partial path in __d_path before figuring out 
that the path is disconnected, so no performance penalty, either.

> In reality, you are comparing "buflen < namelen+1" but spelling it as
> "buflen <= namelen".  I would prefer the full spelling with least room
> for confusion.

I'm fine either way.

> Maybe:
> > +		buflen -= namelen + 1;
> > +		buffer -= namelen + 1;
> > +		memcpy(buffer+1, dentry->d_name.name, namelen);
> > +		*buffer = '/';

That's better, yes.

Thanks,
Andreas

  reply	other threads:[~2007-02-15  2:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-03  3:23 Andreas Gruenbacher
2007-02-05  0:15 ` Neil Brown
2007-02-15  2:43   ` Andreas Gruenbacher [this message]
2007-02-05  8:32 ` Andreas Gruenbacher
2007-02-05 18:37   ` [PATCH]Add memory barrier before clear bit in unlock_buffer() Mingming Cao
2007-02-14  8:19   ` [PATCH] Fix d_path for lazy unmounts Andreas Gruenbacher
2007-02-14  8:29     ` Olaf Hering
2007-02-14  8:42       ` Andreas Gruenbacher
2007-02-14 15:37     ` Linus Torvalds
2007-02-14 19:39       ` Andreas Gruenbacher
2007-02-14 22:57         ` Andreas Gruenbacher
2007-02-15  3:13           ` Andreas Gruenbacher
2007-02-17 13:30             ` Andreas Gruenbacher
2007-02-15 12:53           ` Jan Engelhardt
2007-02-15 13:19             ` Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200702141843.49668.agruen@suse.de \
    --to=agruen@suse.de \
    --cc=akpm@osdl.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=tonyj@suse.de \
    --subject='Re: [PATCH] Fix d_path for lazy unmounts' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).