All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Shukun Tan <tanshukun1@huawei.com>
Cc: b6da310b-e633-9f74-f7af-7791d803aaf5@huawei.com,
	Zhou Wang <wangzhou1@hisilicon.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH v2] crypto: hisilicon - allow smaller reads in debugfs
Date: Fri, 05 Jun 2020 09:13:24 +0000	[thread overview]
Message-ID: <20200605091324.GR30374@kadam> (raw)
In-Reply-To: <3e9b204f-ef53-31c5-afcd-e0791224c2b0@huawei.com>

On Fri, Jun 05, 2020 at 09:19:53AM +0800, Shukun Tan wrote:
> Hi Dan,
> 
> On 2020/6/2 21:54, Dan Carpenter wrote:
> > Originally this code rejected any read less than 256 bytes.  There
> > is no need for this artificial limit.  We should just use the normal
> > helper functions to read a string from the kernel.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: Use simple_read_from_buffer().  The v1 was slightly half arsed
> > because I left the original check for:
> > 
> > 	if (*pos)
> > 		return 0;
> > 
> > So it could result in partial reads.  The new code means that if you
> > want to read the buffer one byte at a time, that's fine or if you want
> > to read it in one 256 byte chunk that's also fine.  Plus it deletes 21
> > lines of code and is a lot cleaner.
> > 
> 
> In fact, In our original design, we do not hope the user do the partial reads.
> Thank you for your work, but I still insist on adding this limit.

This not how POSIX filesystems work...  :(  Last time you said that this
literally breaks cat.

This doesn't break anything if the user chooses not to read a single
byte at a time.  That's obviously a crazy way to read a file.  It just
allows them to if they want.  Or if they want to read 256 bytes at a
time then that also works.  My patch makes *everything* work.

regards,
dan carpenter

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Shukun Tan <tanshukun1@huawei.com>
Cc: b6da310b-e633-9f74-f7af-7791d803aaf5@huawei.com,
	Zhou Wang <wangzhou1@hisilicon.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH v2] crypto: hisilicon - allow smaller reads in debugfs
Date: Fri, 5 Jun 2020 12:13:24 +0300	[thread overview]
Message-ID: <20200605091324.GR30374@kadam> (raw)
In-Reply-To: <3e9b204f-ef53-31c5-afcd-e0791224c2b0@huawei.com>

On Fri, Jun 05, 2020 at 09:19:53AM +0800, Shukun Tan wrote:
> Hi Dan,
> 
> On 2020/6/2 21:54, Dan Carpenter wrote:
> > Originally this code rejected any read less than 256 bytes.  There
> > is no need for this artificial limit.  We should just use the normal
> > helper functions to read a string from the kernel.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: Use simple_read_from_buffer().  The v1 was slightly half arsed
> > because I left the original check for:
> > 
> > 	if (*pos)
> > 		return 0;
> > 
> > So it could result in partial reads.  The new code means that if you
> > want to read the buffer one byte at a time, that's fine or if you want
> > to read it in one 256 byte chunk that's also fine.  Plus it deletes 21
> > lines of code and is a lot cleaner.
> > 
> 
> In fact, In our original design, we do not hope the user do the partial reads.
> Thank you for your work, but I still insist on adding this limit.

This not how POSIX filesystems work...  :(  Last time you said that this
literally breaks cat.

This doesn't break anything if the user chooses not to read a single
byte at a time.  That's obviously a crazy way to read a file.  It just
allows them to if they want.  Or if they want to read 256 bytes at a
time then that also works.  My patch makes *everything* work.

regards,
dan carpenter

  reply	other threads:[~2020-06-05  9:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-02 13:54 [PATCH v2] crypto: hisilicon - allow smaller reads in debugfs Dan Carpenter
2020-06-02 13:54 ` Dan Carpenter
2020-06-05  1:19 ` Shukun Tan
2020-06-05  1:19   ` Shukun Tan
2020-06-05  9:13   ` Dan Carpenter [this message]
2020-06-05  9:13     ` Dan Carpenter
2020-06-18  7:56 ` Herbert Xu
2020-06-18  7:56   ` Herbert Xu

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=20200605091324.GR30374@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=b6da310b-e633-9f74-f7af-7791d803aaf5@huawei.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tanshukun1@huawei.com \
    --cc=wangzhou1@hisilicon.com \
    /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.