public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Pavel Begunkov <[email protected]>,
	Dmitry Kadashev <[email protected]>,
	[email protected]
Subject: Re: Use of disowned struct filename after 3c5499fa56f5?
Date: Thu, 5 Nov 2020 13:49:40 -0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 11/5/20 1:35 PM, Pavel Begunkov wrote:
> On 05/11/2020 20:26, Jens Axboe wrote:
>> On 11/5/20 1:04 PM, Pavel Begunkov wrote:
>>> On 05/11/2020 19:37, Jens Axboe wrote:
>>>> On 11/5/20 7:55 AM, Pavel Begunkov wrote:
>>>>> On 05/11/2020 14:22, Pavel Begunkov wrote:
>>>>>> On 05/11/2020 12:36, Dmitry Kadashev wrote:
>>>>> Hah, basically filename_parentat() returns back the passed in filename if not
>>>>> an error, so @oldname and @from are aliased, then in the end for retry path
>>>>> it does.
>>>>>
>>>>> ```
>>>>> put(from);
>>>>> goto retry;
>>>>> ```
>>>>>
>>>>> And continues to use oldname. The same for to/newname.
>>>>> Looks buggy to me, good catch!
>>>>
>>>> How about we just cleanup the return path? We should only put these names
>>>> when we're done, not for the retry path. Something ala the below - untested,
>>>> I'll double check, test, and see if it's sane.
>>>
>>> Retry should work with a comment below because it uses @oldname
>>> knowing that it aliases to @from, which still have a refcount, but I
>>> don't like this implicit ref passing. If someone would change
>>> filename_parentat() to return a new filename, that would be a nasty
>>> bug.
>>
>> Not a huge fan of how that works either, but I'm not in this to rewrite
>> namei.c...
> 
> There are 6 call sites including do_renameat2(), a separate patch would
> change just ~15-30 lines, doesn't seem like a big rewrite.

It just seems like an utterly pointless exercise to me, something you'd
go through IFF you're changing filename_parentat() to return a _new_
entry instead of just the same one. And given that this isn't the only
callsite, there's precedence there for it working like that. I'd
essentially just be writing useless code.

I can add a comment about it, but again, there are 6 other call sites.

-- 
Jens Axboe


  reply	other threads:[~2020-11-05 20:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-05 12:36 Use of disowned struct filename after 3c5499fa56f5? Dmitry Kadashev
2020-11-05 14:22 ` Pavel Begunkov
2020-11-05 14:26   ` Pavel Begunkov
2020-11-05 14:55   ` Pavel Begunkov
2020-11-05 19:37     ` Jens Axboe
2020-11-05 20:04       ` Pavel Begunkov
2020-11-05 20:18         ` Pavel Begunkov
2020-11-05 20:26         ` Jens Axboe
2020-11-05 20:35           ` Pavel Begunkov
2020-11-05 20:49             ` Jens Axboe [this message]
2020-11-05 20:57               ` Pavel Begunkov
2020-11-05 21:12                 ` Jens Axboe
2020-11-06 10:08                 ` Dmitry Kadashev
2020-11-06 12:49                   ` Pavel Begunkov
2020-11-06 13:15                     ` Dmitry Kadashev
2020-11-06 13:27                       ` Pavel Begunkov
2020-11-06 13:35                         ` Dmitry Kadashev

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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox