[PATCH] staging: lustre: Remove unnecessary explicit comparisons

Cristina Georgiana Opriceana cristina.opriceana at gmail.com
Sun Oct 18 14:13:40 EEST 2015


On Sun, Oct 18, 2015 at 1:05 PM, Cristina-Gabriela Moraru
<cristina.moraru09 at gmail.com> wrote:
>
>
> 2015-10-18 11:26 GMT+03:00 Cristina Georgiana Opriceana
> <cristina.opriceana at gmail.com>:
>>
>> On Sun, Oct 18, 2015 at 2:00 AM, Cristina Moraru
>> <cristina.moraru09 at gmail.com> wrote:
>> >
>> > Remove explicit comparisons with 0 or NULL in order to
>> > provide efficiency.
>>
>> Did you do this with coccinelle? In case you did not, you might
>> consider something like this to do the replacement:
>>
>> @replace_rule@
>> identifier e;
>> @@
>>
>> (
>> - e == 0
>> + !e
>> |
>> - e == NULL
>> + !e
>> )
>>
>
> I wrote a similar script but with 'expression' keyword instead of
> 'identifier' and could not be applied because of one line in file: int
> stopped = !!(rq_state & RQ_STOP); that confused coccinelle parsing and
> produced an error. Now it works. Thanks !

Yes, I think there's a way to exclude some lines from substitution,
but I'm not sure how. It's a thing to investigate.

>
>>
>> > Signed-off-by: Cristina Moraru <cristina.moraru09 at gmail.com>
>> > ---
>> >  drivers/staging/lustre/lustre/mgc/mgc_request.c | 60
>> > ++++++++++++-------------
>> >  1 file changed, 30 insertions(+), 30 deletions(-)
>> >
>> > diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> > b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> > index 780ea81..ab8c5fa 100644
>> > --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> > +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> > @@ -172,7 +172,7 @@ struct config_llog_data *config_log_find(char
>> > *logname,
>> >                         continue;
>> >
>> >                 /* instance may be NULL, should check name */
>> > -               if (strcmp(logname, cld->cld_logname) == 0) {
>> > +               if (!strcmp(logname, cld->cld_logname)) {
>> >                         found = cld;
>> >                         break;
>> >                 }
>> > @@ -300,7 +300,7 @@ static int config_log_add(struct obd_device *obd,
>> > char *logname,
>> >          * <fsname>-sptlrpc. multiple regular logs may share one sptlrpc
>> > log.
>> >          */
>> >         ptr = strrchr(logname, '-');
>> > -       if (ptr == NULL || ptr - logname > 8) {
>> > +       if (!ptr || ptr - logname > 8) {
>> >                 CERROR("logname %s is too long\n", logname);
>> >                 return -EINVAL;
>> >         }
>> > @@ -309,7 +309,7 @@ static int config_log_add(struct obd_device *obd,
>> > char *logname,
>> >         strcpy(seclogname + (ptr - logname), "-sptlrpc");
>> >
>> >         sptlrpc_cld = config_log_find(seclogname, NULL);
>> > -       if (sptlrpc_cld == NULL) {
>> > +       if (!sptlrpc_cld) {
>> >                 sptlrpc_cld = do_config_log_add(obd, seclogname,
>> >                                                 CONFIG_T_SPTLRPC, NULL,
>> > NULL);
>> >                 if (IS_ERR(sptlrpc_cld)) {
>> > @@ -376,7 +376,7 @@ static int config_log_end(char *logname, struct
>> > config_llog_instance *cfg)
>> >         int rc = 0;
>> >
>> >         cld = config_log_find(logname, cfg);
>> > -       if (cld == NULL)
>> > +       if (!cld)
>> >                 return -ENOENT;
>> >
>> >         mutex_lock(&cld->cld_lock);
>> > @@ -451,7 +451,7 @@ int lprocfs_mgc_rd_ir_state(struct seq_file *m, void
>> > *data)
>> >
>> >         spin_lock(&config_list_lock);
>> >         list_for_each_entry(cld, &config_llog_list, cld_list_chain) {
>> > -               if (cld->cld_recover == NULL)
>> > +               if (!cld->cld_recover)
>> >                         continue;
>> >                 seq_printf(m,  "    - { client: %s, nidtbl_version: %u
>> > }\n",
>> >                                cld->cld_logname,
>> > @@ -482,7 +482,7 @@ static void do_requeue(struct config_llog_data *cld)
>> >            export which is being disconnected. Take the client
>> >            semaphore to make the check non-racy. */
>> >         down_read(&cld->cld_mgcexp->exp_obd->u.cli.cl_sem);
>> > -       if (cld->cld_mgcexp->exp_obd->u.cli.cl_conn_count != 0) {
>> > +       if (cld->cld_mgcexp->exp_obd->u.cli.cl_conn_count) {
>> >                 CDEBUG(D_MGC, "updating log %s\n", cld->cld_logname);
>> >                 mgc_process_log(cld->cld_mgcexp->exp_obd, cld);
>> >         } else {
>> > @@ -683,7 +683,7 @@ static int mgc_precleanup(struct obd_device *obd,
>> > enum obd_cleanup_stage stage)
>> >                         wait_for_completion(&rq_exit);
>> >                 obd_cleanup_client_import(obd);
>> >                 rc = mgc_llog_fini(NULL, obd);
>> > -               if (rc != 0)
>> > +               if (rc)
>> >                         CERROR("failed to cleanup llogging
>> > subsystems\n");
>> >                 break;
>> >         }
>> > @@ -879,7 +879,7 @@ static int mgc_enqueue(struct obd_export *exp,
>> > struct lov_stripe_md *lsm,
>> >         req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp),
>> >                                         &RQF_LDLM_ENQUEUE,
>> > LUSTRE_DLM_VERSION,
>> >                                         LDLM_ENQUEUE);
>> > -       if (req == NULL)
>> > +       if (!req)
>> >                 return -ENOMEM;
>> >
>> >         req_capsule_set_size(&req->rq_pill, &RMF_DLM_LVB, RCL_SERVER,
>> > 0);
>> > @@ -917,7 +917,7 @@ static int mgc_target_register(struct obd_export
>> > *exp,
>> >         req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp),
>> >                                         &RQF_MGS_TARGET_REG,
>> > LUSTRE_MGS_VERSION,
>> >                                         MGS_TARGET_REG);
>> > -       if (req == NULL)
>> > +       if (!req)
>> >                 return -ENOMEM;
>> >
>> >         req_mti = req_capsule_client_get(&req->rq_pill,
>> > &RMF_MGS_TARGET_INFO);
>> > @@ -988,7 +988,7 @@ static int mgc_set_info_async(const struct lu_env
>> > *env, struct obd_export *exp,
>> >                  * if flavor has been set previously, check the asking
>> > flavor
>> >                  * must match the existing one.
>> >                  */
>> > -               if (vallen == 0) {
>> > +               if (!vallen) {
>> >                         if (cli->cl_flvr_mgc.sf_rpc !=
>> > SPTLRPC_FLVR_INVALID)
>> >                                 return 0;
>> >                         val = "null";
>> > @@ -1008,7 +1008,7 @@ static int mgc_set_info_async(const struct lu_env
>> > *env, struct obd_export *exp,
>> >                 if (cli->cl_flvr_mgc.sf_rpc == SPTLRPC_FLVR_INVALID) {
>> >                         cli->cl_flvr_mgc = flvr;
>> >                 } else if (memcmp(&cli->cl_flvr_mgc, &flvr,
>> > -                                 sizeof(flvr)) != 0) {
>> > +                                 sizeof(flvr))) {
>> >                         char    str[20];
>> >
>> >                         sptlrpc_flavor2name(&cli->cl_flvr_mgc,
>> > @@ -1138,9 +1138,9 @@ static int mgc_apply_recover_logs(struct
>> > obd_device *mgc,
>> >                 entry = (typeof(entry))(data + off);
>> >
>> >                 /* sanity check */
>> > -               if (entry->mne_nid_type != 0) /* only support type 0 for
>> > ipv4 */
>> > +               if (entry->mne_nid_type) /* only support type 0 for ipv4
>> > */
>> >                         break;
>> > -               if (entry->mne_nid_count == 0) /* at least one nid entry
>> > */
>> > +               if (!entry->mne_nid_count) /* at least one nid entry */
>> >                         break;
>> >                 if (entry->mne_nid_size != sizeof(lnet_nid_t))
>> >                         break;
>> > @@ -1191,7 +1191,7 @@ static int mgc_apply_recover_logs(struct
>> > obd_device *mgc,
>> >                 /* lustre-OST0001-osc-<instance #> */
>> >                 strcpy(obdname, cld->cld_logname);
>> >                 cname = strrchr(obdname, '-');
>> > -               if (cname == NULL) {
>> > +               if (!cname) {
>> >                         CERROR("mgc %s: invalid logname %s\n",
>> >                                mgc->obd_name, obdname);
>> >                         break;
>> > @@ -1208,7 +1208,7 @@ static int mgc_apply_recover_logs(struct
>> > obd_device *mgc,
>> >
>> >                 /* find the obd by obdname */
>> >                 obd = class_name2obd(obdname);
>> > -               if (obd == NULL) {
>> > +               if (!obd) {
>> >                         CDEBUG(D_INFO, "mgc %s: cannot find obdname
>> > %s\n",
>> >                                mgc->obd_name, obdname);
>> >                         rc = 0;
>> > @@ -1223,7 +1223,7 @@ static int mgc_apply_recover_logs(struct
>> > obd_device *mgc,
>> >                 uuid = buf + pos;
>> >
>> >                 down_read(&obd->u.cli.cl_sem);
>> > -               if (obd->u.cli.cl_import == NULL) {
>> > +               if (!obd->u.cli.cl_import) {
>> >                         /* client does not connect to the OST yet */
>> >                         up_read(&obd->u.cli.cl_sem);
>> >                         rc = 0;
>> > @@ -1253,7 +1253,7 @@ static int mgc_apply_recover_logs(struct
>> > obd_device *mgc,
>> >
>> >                 rc = -ENOMEM;
>> >                 lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
>> > -               if (lcfg == NULL) {
>> > +               if (!lcfg) {
>> >                         CERROR("mgc: cannot allocate memory\n");
>> >                         break;
>> >                 }
>> > @@ -1301,18 +1301,18 @@ static int mgc_process_recover_log(struct
>> > obd_device *obd,
>> >          * small and CONFIG_READ_NRPAGES will be used.
>> >          */
>> >         nrpages = CONFIG_READ_NRPAGES;
>> > -       if (cfg->cfg_last_idx == 0) /* the first time */
>> > +       if (!cfg->cfg_last_idx) /* the first time */
>> >                 nrpages = CONFIG_READ_NRPAGES_INIT;
>> >
>> >         pages = kcalloc(nrpages, sizeof(*pages), GFP_NOFS);
>> > -       if (pages == NULL) {
>> > +       if (!pages) {
>> >                 rc = -ENOMEM;
>> >                 goto out;
>> >         }
>> >
>> >         for (i = 0; i < nrpages; i++) {
>> >                 pages[i] = alloc_page(GFP_IOFS);
>> > -               if (pages[i] == NULL) {
>> > +               if (!pages[i]) {
>> >                         rc = -ENOMEM;
>> >                         goto out;
>> >                 }
>> > @@ -1323,7 +1323,7 @@ again:
>> >         LASSERT(mutex_is_locked(&cld->cld_lock));
>> >         req = ptlrpc_request_alloc(class_exp2cliimp(cld->cld_mgcexp),
>> >                                    &RQF_MGS_CONFIG_READ);
>> > -       if (req == NULL) {
>> > +       if (!req) {
>> >                 rc = -ENOMEM;
>> >                 goto out;
>> >         }
>> > @@ -1349,7 +1349,7 @@ again:
>> >         /* allocate bulk transfer descriptor */
>> >         desc = ptlrpc_prep_bulk_imp(req, nrpages, 1, BULK_PUT_SINK,
>> >                                     MGS_BULK_PORTAL);
>> > -       if (desc == NULL) {
>> > +       if (!desc) {
>> >                 rc = -ENOMEM;
>> >                 goto out;
>> >         }
>> > @@ -1387,7 +1387,7 @@ again:
>> >                 goto out;
>> >         }
>> >
>> > -       if (ealen == 0) { /* no logs transferred */
>> > +       if (!ealen) { /* no logs transferred */
>> >                 if (!eof)
>> >                         rc = -EINVAL;
>> >                 goto out;
>> > @@ -1425,12 +1425,12 @@ out:
>> >         if (req)
>> >                 ptlrpc_req_finished(req);
>> >
>> > -       if (rc == 0 && !eof)
>> > +       if (!rc && !eof)
>> >                 goto again;
>> >
>> >         if (pages) {
>> >                 for (i = 0; i < nrpages; i++) {
>> > -                       if (pages[i] == NULL)
>> > +                       if (!pages[i])
>> >                                 break;
>> >                         __free_page(pages[i]);
>> >                 }
>> > @@ -1543,7 +1543,7 @@ int mgc_process_log(struct obd_device *mgc, struct
>> > config_llog_data *cld)
>> >         rcl = mgc_enqueue(mgc->u.cli.cl_mgc_mgsexp, NULL, LDLM_PLAIN,
>> > NULL,
>> >                           LCK_CR, &flags, NULL, NULL, NULL,
>> >                           cld, 0, NULL, &lockh);
>> > -       if (rcl == 0) {
>> > +       if (!rcl) {
>> >                 /* Get the cld, it will be released in mgc_blocking_ast.
>> > */
>> >                 config_log_get(cld);
>> >                 rc = ldlm_lock_set_data(&lockh, (void *)cld);
>> > @@ -1560,7 +1560,7 @@ int mgc_process_log(struct obd_device *mgc, struct
>> > config_llog_data *cld)
>> >
>> >         if (cld_is_recover(cld)) {
>> >                 rc = 0; /* this is not a fatal error for recover log */
>> > -               if (rcl == 0)
>> > +               if (!rcl)
>> >                         rc = mgc_process_recover_log(mgc, cld);
>> >         } else {
>> >                 rc = mgc_process_cfg_log(mgc, cld, rcl != 0);
>> > @@ -1631,7 +1631,7 @@ static int mgc_process_config(struct obd_device
>> > *obd, u32 len, void *buf)
>> >                 if (rc)
>> >                         break;
>> >                 cld = config_log_find(logname, cfg);
>> > -               if (cld == NULL) {
>> > +               if (!cld) {
>> >                         rc = -ENOENT;
>> >                         break;
>> >                 }
>> > @@ -1642,7 +1642,7 @@ static int mgc_process_config(struct obd_device
>> > *obd, u32 len, void *buf)
>> >                 cld->cld_cfg.cfg_flags |= CFG_F_COMPAT146;
>> >
>> >                 rc = mgc_process_log(obd, cld);
>> > -               if (rc == 0 && cld->cld_recover != NULL) {
>> > +               if (!rc && cld->cld_recover) {
>> >                         if (OCD_HAS_FLAG(&obd->u.cli.cl_import->
>> >                                          imp_connect_data, IMP_RECOV)) {
>> >                                 rc = mgc_process_log(obd,
>> > cld->cld_recover);
>> > @@ -1656,7 +1656,7 @@ static int mgc_process_config(struct obd_device
>> > *obd, u32 len, void *buf)
>> >                                 CERROR("Cannot process recover llog
>> > %d\n", rc);
>> >                 }
>> >
>> > -               if (rc == 0 && cld->cld_params != NULL) {
>> > +               if (!rc && cld->cld_params) {
>> >                         rc = mgc_process_log(obd, cld->cld_params);
>> >                         if (rc == -ENOENT) {
>> >                                 CDEBUG(D_MGC,
>> > --
>> > 1.9.1
>> >
>> >


More information about the firefly mailing list