All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: Wenbo Zhang <ethercflow@gmail.com>
Cc: "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"ast@kernel.org" <ast@kernel.org>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	"andrii.nakryiko@gmail.com" <andrii.nakryiko@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next v12 1/2] bpf: add new helper get_file_path for mapping a file descriptor to a pathname
Date: Tue, 17 Dec 2019 06:33:12 +0000	[thread overview]
Message-ID: <fecf759e-2941-3920-82d5-45a556f4dd1d@fb.com> (raw)
In-Reply-To: <CABtjQmZcbhcab0a7ksuggg3ZoDwM5s3ucjeA_baPTpAJQvKQLA@mail.gmail.com>



On 12/16/19 10:26 PM, Wenbo Zhang wrote:
>>> + *           - a regular full path (include mountable fs eg: /proc, /sys)
>>> + *           - a regular full path with "(deleted)" at the end.
> 
>> Let us say with " (deleted)" is appended to be consistent with comments
>> in d_path() and is more clear to user what the format will looks like.
> 
> Thank you, I'll fix this.
> 
>>> +     ret = strlen(p);
>>> +     memmove(dst, p, ret);
>>> +     dst[ret++] = '\0';
> 
>> nit: you could do memmove(dst, p, ret + 1)?
> 
> I did with `dst[ret++]='\0';`  to return value length including
> trailing '\0'. as you mentioned below:
> 
>>> +     fput(f);
>>> +     return ret;
> 
>> The description says the return value length including trailing '\0'.
>> The above 'ret' does not include trailing '\0'.
> 
> It seems `[ret++]` not very clear to read and '\0' can be done by
> `memmove`. I think I'll refactor to
> 
> ```
> ret = strlen(p) + 1;
> memmove(dst, p, ret);
> fput(f);
> return ret;
> ```
> 
> Is this better?

Ah, I missed ret++ in dst[ret++]. Indeed the above code is better.

  reply	other threads:[~2019-12-17  6:33 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-19 13:27 [PATCH bpf-next v10 0/2] bpf: adding get_file_path helper Wenbo Zhang
2019-11-19 13:27 ` [PATCH bpf-next v10 1/2] bpf: add new helper get_file_path for mapping a file descriptor to a pathname Wenbo Zhang
2019-11-23  3:18   ` Alexei Starovoitov
2019-11-23  4:43     ` Al Viro
2019-11-23  4:51     ` Al Viro
2019-11-23  5:19       ` Alexei Starovoitov
2019-11-23  5:35         ` Al Viro
2019-11-23  6:04           ` Alexei Starovoitov
2019-12-13 19:51             ` Brendan Gregg
2019-12-05  4:20   ` [PATCH bpf-next v11 0/2] bpf: adding get_file_path helper Wenbo Zhang
2019-12-05  4:20     ` [PATCH bpf-next v11 1/2] bpf: add new helper get_file_path for mapping a file descriptor to a pathname Wenbo Zhang
2019-12-05  7:19       ` Alexei Starovoitov
2019-12-05  9:47         ` Wenbo Zhang
2019-12-15  4:01       ` [PATCH bpf-next v12 0/2] bpf: adding get_file_path helper Wenbo Zhang
2019-12-15  4:01         ` [PATCH bpf-next v12 1/2] bpf: add new helper get_file_path for mapping a file descriptor to a pathname Wenbo Zhang
2019-12-15 16:05           ` Yonghong Song
2019-12-17  6:26             ` Wenbo Zhang
2019-12-17  6:33               ` Yonghong Song [this message]
2019-12-15 16:10           ` Yonghong Song
2019-12-17  6:27             ` Wenbo Zhang
2019-12-16 22:09           ` Brendan Gregg
2019-12-17  4:05             ` Wenbo Zhang
2019-12-17  9:47           ` [PATCH bpf-next v13 0/2] bpf: adding get_fd_path helper Wenbo Zhang
2019-12-17  9:47             ` [PATCH bpf-next v13 1/2] bpf: add new helper get_fd_path for mapping a file descriptor to a pathname Wenbo Zhang
2019-12-17 16:29               ` Yonghong Song
2019-12-17 19:39                 ` Daniel Borkmann
2019-12-18  0:11                   ` Wenbo Zhang
2019-12-18  0:06                 ` Wenbo Zhang
2019-12-18  0:56               ` [PATCH bpf-next v14 0/2] bpf: adding get_fd_path helper Wenbo Zhang
2019-12-18  0:56                 ` [PATCH bpf-next v14 1/2] bpf: add new helper get_fd_path for mapping a file descriptor to a pathname Wenbo Zhang
2019-12-18  3:27                   ` Yonghong Song
2019-12-19 16:14                   ` Daniel Borkmann
2019-12-20  3:35                     ` Wenbo Zhang
2020-01-16  8:59                       ` Jiri Olsa
2020-02-10  4:43                         ` Brendan Gregg
2020-02-11  0:01                           ` Daniel Borkmann
2020-02-12 15:21                             ` Jiri Olsa
2020-06-01 14:17                               ` Wenbo Zhang
2020-06-01 16:38                                 ` Alexei Starovoitov
2020-06-02  3:04                                   ` Wenbo Zhang
2020-06-02  8:14                                     ` Jiri Olsa
2019-12-18  0:56                 ` [PATCH bpf-next v14 2/2] selftests/bpf: test for bpf_get_fd_path() from tracepoint Wenbo Zhang
2019-12-18  3:27                   ` Yonghong Song
2019-12-17  9:47             ` [PATCH bpf-next v13 " Wenbo Zhang
2019-12-17 16:32               ` Yonghong Song
2019-12-15  4:01         ` [PATCH bpf-next v12 2/2] selftests/bpf: test for bpf_get_file_path() " Wenbo Zhang
2019-12-15 16:24           ` Yonghong Song
2019-12-17  4:01             ` Wenbo Zhang
2019-12-17  4:13               ` Yonghong Song
2019-12-17  9:44                 ` Wenbo Zhang
2019-12-05  4:20     ` [PATCH bpf-next v11 " Wenbo Zhang
2019-11-19 13:27 ` [PATCH bpf-next v10 " Wenbo Zhang

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=fecf759e-2941-3920-82d5-45a556f4dd1d@fb.com \
    --to=yhs@fb.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=ethercflow@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.