mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Pekka J Enberg" <penberg@cs.helsinki.fi>
To: Anton Altaparmakov <aia21@cam.ac.uk>
Cc: linux-ntfs-dev@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: ntfs: remove redundant assignments
Date: Thu, 26 May 2005 09:21:46 +0300	[thread overview]
Message-ID: <courier.42956AFA.00002502@courier.cs.helsinki.fi> (raw)
In-Reply-To: <Pine.LNX.4.60.0505252208120.25834@hermes-1.csi.cam.ac.uk>

Hi Anton, 

At some point in time, I wrote:
> Index: 2.6-mm/fs/ntfs/super.c
> > ===================================================================
> > --- 2.6-mm.orig/fs/ntfs/super.c	2005-05-25 20:51:57.000000000 +0300
> > +++ 2.6-mm/fs/ntfs/super.c	2005-05-25 20:54:02.000000000 +0300
> > @@ -2283,7 +2283,7 @@
> >  	sb->s_flags |= MS_RDONLY | MS_NOATIME | MS_NODIRATIME;
> >  #endif /* ! NTFS_RW */
> >  	/* Allocate a new ntfs_volume and place it in sb->s_fs_info. */
> > -	sb->s_fs_info = kmalloc(sizeof(ntfs_volume), GFP_NOFS);
> > +	sb->s_fs_info = kcalloc(1, sizeof(ntfs_volume), GFP_NOFS);
> >  	vol = NTFS_SB(sb);
> >  	if (!vol) {
> >  		if (!silent)
> > @@ -2292,28 +2292,9 @@
> >  		return -ENOMEM;
> >  	}
> >  	/* Initialize ntfs_volume structure. */
> > -	memset(vol, 0, sizeof(ntfs_volume));
> >  	vol->sb = sb; 
> 
> The above is fine, thanks. 
> 
> > -	vol->upcase = NULL;
> > -	vol->attrdef = NULL;
> > -	vol->mft_ino = NULL;
> > -	vol->mftbmp_ino = NULL;
> >  	init_rwsem(&vol->mftbmp_lock);
> > -#ifdef NTFS_RW
> > -	vol->mftmirr_ino = NULL;
> > -	vol->logfile_ino = NULL;
> > -#endif /* NTFS_RW */
> > -	vol->lcnbmp_ino = NULL;
> >  	init_rwsem(&vol->lcnbmp_lock);
> > -	vol->vol_ino = NULL;
> > -	vol->root_ino = NULL;
> > -	vol->secure_ino = NULL;
> > -	vol->extend_ino = NULL;
> > -#ifdef NTFS_RW
> > -	vol->quota_ino = NULL;
> > -	vol->quota_q_ino = NULL;
> > -#endif /* NTFS_RW */
> > -	vol->nls_map = NULL;

On Wed, 2005-05-25 at 22:10 +0100, Anton Altaparmakov wrote:
> This is not.  memset(0) is not the same as = NULL IMO.  I don't care if 
> the compiler thinks it is the same.  NULL does not have to be 0 so I 
> prefer to initialize pointers explicitly to NULL.  Even more so since this 
> code is not performance critical at all so I prefer clarity here.

I kind of figured out you were doing it on purpose. The fact is, NULL is 
zero on _all_ Linux architectures. If it weren't, we'd have a lot of broken 
code. Let me play the devils advocate here: why do you memset() (now 
kcalloc()) in the first place? 

At some point in time, I wrote:
> > Index: 2.6-mm/fs/ntfs/index.c
> > ===================================================================
> > --- 2.6-mm.orig/fs/ntfs/index.c	2005-05-25 20:51:57.000000000 +0300
> > +++ 2.6-mm/fs/ntfs/index.c	2005-05-25 21:07:38.000000000 +0300
> > @@ -40,16 +40,8 @@
> >  
> >  	ictx = kmem_cache_alloc(ntfs_index_ctx_cache, SLAB_NOFS);
> >  	if (ictx) {
> > +		memset(ictx, 0, sizeof(*ictx));
> >  		ictx->idx_ni = idx_ni;
> > -		ictx->entry = NULL;
> > -		ictx->data = NULL;
> > -		ictx->data_len = 0;
> > -		ictx->is_in_root = 0;
> > -		ictx->ir = NULL;
> > -		ictx->actx = NULL;
> > -		ictx->base_ni = NULL;
> > -		ictx->ia = NULL;
> > -		ictx->page = NULL;

On Wed, 2005-05-25 at 22:10 +0100, Anton Altaparmakov wrote:
> Again, as above, I prefer to have the explicit = NULL instead of a memset.

There's a simple reason why I don't like explicit assignments: it's way too 
easy to forget to initialize something. 

			Pekka 

       reply	other threads:[~2005-05-26  6:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1117044875.9510.2.camel@localhost>
     [not found] ` <Pine.LNX.4.60.0505252208120.25834@hermes-1.csi.cam.ac.uk>
2005-05-26  6:21   ` Pekka J Enberg [this message]
2005-05-26  7:04     ` Al Viro
2005-05-26  8:14       ` [PATCH] ntfs: use struct initializers Pekka J Enberg
2005-05-27 15:45         ` Anton Altaparmakov
2005-05-26  8:39       ` ntfs: remove redundant assignments cutaway
2005-05-27 15:04       ` Anton Altaparmakov

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=courier.42956AFA.00002502@courier.cs.helsinki.fi \
    --to=penberg@cs.helsinki.fi \
    --cc=aia21@cam.ac.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ntfs-dev@lists.sourceforge.net \
    /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

all inboxes | Powered by JetHome®