From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Thu, 19 Nov 2015 01:44:33 +0300
Subject: [Cluster-devel] GFS2: Extract quota data from reservations
 structure (revert 5407e24)
Message-ID: <20151118224433.GA25959@mwanda>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hello Bob Peterson,

This is a semi-automatic email about new static checker warnings.

The patch ae3547b14335: "GFS2: Extract quota data from reservations 
structure (revert 5407e24)" from Oct 26, 2015, leads to the following 
Smatch complaint:

fs/gfs2/quota.c:573 gfs2_quota_hold()
	 error: we previously assumed 'ip->i_qadata' could be null (see line 567)

fs/gfs2/quota.c
   535  int gfs2_qa_alloc(struct gfs2_inode *ip)
   536  {
   537          int error = 0;
   538          struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
   539  
   540          if (sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
   541                  return 0;

Success return here doesn't mean that ->i_qadata was allocated.

I feel like this is the wrong place for this check.  The function name
is "alloc" so it should allocate instead of making policy decisions.  We
do check for GFS2_QUOTA_ON in gfs2_quota_lock() but that check is too
late so we would have oopsed already.  Let's move that check in front of
the the gfs2_quota_hold() call and delete the check from here.

   542  
   543          down_write(&ip->i_rw_mutex);
   544          ip->i_qadata = kmem_cache_zalloc(gfs2_qadata_cachep, GFP_NOFS);
   545          if (!ip->i_qadata)
   546                  error = -ENOMEM;
   547          up_write(&ip->i_rw_mutex);
   548          return error;
   549  }
   550  
   551  void gfs2_qa_delete(struct gfs2_inode *ip, atomic_t *wcount)
   552  {
   553          down_write(&ip->i_rw_mutex);
   554          if (ip->i_qadata && ((wcount == NULL) || (atomic_read(wcount) <= 1))) {
   555                  kmem_cache_free(gfs2_qadata_cachep, ip->i_qadata);
   556                  ip->i_qadata = NULL;
   557          }
   558          up_write(&ip->i_rw_mutex);
   559  }
   560  
   561  int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
   562  {
   563          struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
   564          struct gfs2_quota_data **qd;
   565          int error;
   566	
   567		if (ip->i_qadata == NULL) {
                    ^^^^^^^^^^^^^^^^^^^^
Check for NULL.

   568			error = gfs2_qa_alloc(ip);
   569			if (error)
   570				return error;
   571		}
   572	
   573		qd = ip->i_qadata->qa_qd;
                ^^^^^^^^^^^^^^^^^^^^^^^^
Dereference.

   574	
   575		if (gfs2_assert_warn(sdp, !ip->i_qadata->qa_qd_num) ||

regards,
dan carpenter