On 14/02/2022 02:17, Damien Le Moal wrote:
Introduce the pm8001_ccb_alloc() and pm8001_ccb_free() helpers to replace the typical code patterns: res = pm8001_tag_alloc(pm8001_ha, &ccb_tag); if (res) ... ccb = &pm8001_ha->ccb_info[ccb_tag]; ccb->device = pm8001_ha_dev; ccb->ccb_tag = ccb_tag; ccb->task = task; ccb->n_elem = 0;A
nit: stray 'A' character
and ccb->task = NULL; ccb->ccb_tag = PM8001_INVALID_TAG; pm8001_tag_free(pm8001_ha, tag);
Isn't it possible to do a "memset struct members", such that we order the members specifically that we can memset(0) a known region? The advantage is that adding/removing a member does not require much code to be touched and possibly missed.
With the simpler function calls: ccb = pm8001_ccb_alloc(pm8001_ha, pm8001_ha_dev, task); if (!ccb) ... and pm8001_ccb_free(pm8001_ha, ccb); The pm8001_ccb_alloc() helper ensures that all fields of the ccb info structure for the newly allocated tag are all initialized, except the buf_prd field. The pm8001_ccb_free() helper clears the initialized fields and the ccb tag to ensure that iteration over the adapter ccb_info array detects ccbs that are in use.
All call site of the pm8001_tag_alloc() function that use a ccb info associated with an allocated tag are converted to use the new helpers. Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> ---
The comments are not a show stopper, so: Reviewed-by: John Garry <john.garry@xxxxxxxxxx>
