xfs
[Top] [All Lists]

Re: [PATCH 2/4] xfs: Add pquota fields where gquota is used

To: Jeff Liu <jeff.liu@xxxxxxxxxx>
Subject: Re: [PATCH 2/4] xfs: Add pquota fields where gquota is used
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 17 Jul 2012 03:36:45 -0400
Cc: xfs@xxxxxxxxxxx, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Ben Myers <bpm@xxxxxxx>, Mark Tinguely <tinguely@xxxxxxx>, Chandra Seetharaman <sekharan@xxxxxxxxxx>
In-reply-to: <4FFFDDE5.8010403@xxxxxxxxxx>
References: <4FFFDDE5.8010403@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
>               if (error) {
>                       if (uip)
>                               IRELE(uip);
> @@ -1421,9 +1474,24 @@ xfs_qm_init_quotainos(
>                       return XFS_ERROR(error);
>               }
>       }
> +     /* Why not define a XFS_SB_PQUOTINO? */
> +     if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
> +             error = xfs_qm_qino_alloc(mp, &pip,
> +                                       sbflags | XFS_SB_GQUOTINO,
> +                                       flags | XFS_QMOPT_PQUOTA);
> +             if (error) {
> +                     if (uip)
> +                             IRELE(uip);
> +                     if (gip)
> +                             IRELE(gip);
> +
> +                     return XFS_ERROR(error);

It would probably be good to have return labels here so that each
IRELE statements only needs to be written once, e.g.

out_rele_uip:
        if (uip)
                IRELE(uip);
out_rele_gip:
        if (gip)
                IRELE(gip);
out:
        return XFS_ERROR(error);

> +                     if ((error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)uid,
> +                                               XFS_DQ_USER,
> +                                               XFS_QMOPT_DQALLOC |
> +                                               XFS_QMOPT_DOWARN, &uq))) {

Please move assignments out of conditionals for all code touched, e.g.:

                        error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)uid,
                                             XFS_DQ_USER,
                                             XFS_QMOPT_DQALLOC |
                                             XFS_QMOPT_DOWARN, &uq);
                        if (error) {

Otherwise this looks good to me.

<Prev in Thread] Current Thread [Next in Thread>