All of lore.kernel.org
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: walter harms <wharms@bfs.de>, Julia Lawall <julia.lawall@lip6.fr>,
	Michael Halcrow <mhalcrow@google.com>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-ext4@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [patch] ext4 crypto: testing the wrong variable
Date: Sat, 11 Apr 2015 13:48:11 +0000	[thread overview]
Message-ID: <20150411134811.GI6540@thunk.org> (raw)
In-Reply-To: <20150408122251.GJ10964@mwanda>

On Wed, Apr 08, 2015 at 03:22:51PM +0300, Dan Carpenter wrote:
> > why test *buf = NULL ? xfree() can handle this.
> > 
> > the question is do programm depend on *buf=NULL.
> > In case of IS_ERR(*buf) *buf will be left unchanged
> > and later prgramms may things there is a buffer
> > available ?
> 
> Good point.  That IS_ERR() check is going to cause all kinds of future
> bugs.

Yes, it's not needed at all, so I'll just remove it.  I'm also going
to be fixing the whole fname_crypto_alloc_buffer() and
fname_crypto_free_buffer() so the calling convention isn't as
horrendous.  So basically, instead of

int ext4_fname_crypto_alloc_buffer(struct ext4_fname_crypto_ctx *ctx,
				   unsigned char **obuf, u32 *olen, u32 ilen);
void ext4_fname_crypto_free_buffer(void **buf);

it will be:

int ext4_fname_crypto_alloc_buffer(struct ext4_fname_crypto_ctx *ctx,
				   struct ext4_str *crypto_str, u32 ilen);
void ext4_fname_crypto_free_buffer(struct ext4_str *crypto_str);

(And BTW, this isn't appropriate for kernel-janitors since this code
isn't upstream yet)

							- Ted

WARNING: multiple messages have this Message-ID (diff)
From: Theodore Ts'o <tytso@mit.edu>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: walter harms <wharms@bfs.de>, Julia Lawall <julia.lawall@lip6.fr>,
	Michael Halcrow <mhalcrow@google.com>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-ext4@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [patch] ext4 crypto: testing the wrong variable
Date: Sat, 11 Apr 2015 09:48:11 -0400	[thread overview]
Message-ID: <20150411134811.GI6540@thunk.org> (raw)
In-Reply-To: <20150408122251.GJ10964@mwanda>

On Wed, Apr 08, 2015 at 03:22:51PM +0300, Dan Carpenter wrote:
> > why test *buf == NULL ? xfree() can handle this.
> > 
> > the question is do programm depend on *buf=NULL.
> > In case of IS_ERR(*buf) *buf will be left unchanged
> > and later prgramms may things there is a buffer
> > available ?
> 
> Good point.  That IS_ERR() check is going to cause all kinds of future
> bugs.

Yes, it's not needed at all, so I'll just remove it.  I'm also going
to be fixing the whole fname_crypto_alloc_buffer() and
fname_crypto_free_buffer() so the calling convention isn't as
horrendous.  So basically, instead of

int ext4_fname_crypto_alloc_buffer(struct ext4_fname_crypto_ctx *ctx,
				   unsigned char **obuf, u32 *olen, u32 ilen);
void ext4_fname_crypto_free_buffer(void **buf);

it will be:

int ext4_fname_crypto_alloc_buffer(struct ext4_fname_crypto_ctx *ctx,
				   struct ext4_str *crypto_str, u32 ilen);
void ext4_fname_crypto_free_buffer(struct ext4_str *crypto_str);

(And BTW, this isn't appropriate for kernel-janitors since this code
isn't upstream yet)

							- Ted

  reply	other threads:[~2015-04-11 13:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-08  8:53 [patch] ext4 crypto: testing the wrong variable Dan Carpenter
2015-04-08  8:53 ` Dan Carpenter
2015-04-08  9:51 ` Julia Lawall
2015-04-08  9:51   ` Julia Lawall
2015-04-08 11:51   ` walter harms
2015-04-08 11:51     ` walter harms
2015-04-08 12:22     ` Dan Carpenter
2015-04-08 12:22       ` Dan Carpenter
2015-04-11 13:48       ` Theodore Ts'o [this message]
2015-04-11 13:48         ` Theodore Ts'o
2015-04-15  9:26         ` Dan Carpenter
2015-04-15  9:26           ` Dan Carpenter
2015-04-08 18:15 ` Andreas Dilger
2015-04-08 18:15   ` Andreas Dilger

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=20150411134811.GI6540@thunk.org \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=dan.carpenter@oracle.com \
    --cc=julia.lawall@lip6.fr \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=mhalcrow@google.com \
    --cc=wharms@bfs.de \
    /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.