On Sun, Sep 10, 2017 at 7:39 PM, Yasunori Goto <y-g...@jp.fujitsu.com> wrote:
>> > +
>> > + if (!found)
>> > + goto err_found;
>> > +
>> > + for (i = 0; i < found; i++) {
>> > + const char *devname = ndctl_dimm_get_devname(dimms[i]);
>> > +
>> > + jobj = json_object_new_string(devname);
>> > + if (!jobj)
>> > + break;
>> > + json_object_array_add(jdimms, jobj);
>> > + }
>>
>> I have one comment.
>>
>> If output is sorted by device number here,
>> it may be friendly for users.
>> (Especially, # of interleave way becomes huge in future...)
>>
>> Other things are looks good for me.
>
> I made a patch to make sorting.
> I hope it will help....
>
> -------
> To make user friendly, it is desirable that ndctl sorts by dimm's number,
> when output dimms which have badblocks.
>
> Signed-off-by: Yasunori Goto <y-g...@jp.fujitsu.com>
>
> ---
> util/json.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/util/json.c b/util/json.c
> index 9b7773e..dac8692 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -366,6 +366,21 @@ struct json_object *util_daxctl_region_to_json(struct
> daxctl_region *region,
> return NULL;
> }
>
> +static int compare_dimm_number(const void *p1, const void *p2)
> +{
> + struct ndctl_dimm *dimm1 = *(struct ndctl_dimm **)p1;
> + struct ndctl_dimm *dimm2 = *(struct ndctl_dimm **)p2;
> + const char *dimm1_name = ndctl_dimm_get_devname(dimm1);
> + const char *dimm2_name = ndctl_dimm_get_devname(dimm2);
> + int num1, num2;
> +
> + sscanf(dimm1_name, "nmem%d", &num1);
> + sscanf(dimm2_name, "nmem%d", &num2);
> +
> + return num1 - num2;
> +
> +}
> +
> static struct json_object *badblocks_to_jdimms(struct ndctl_region *region,
> unsigned long long addr, unsigned long len)
> {
> @@ -399,6 +414,8 @@ static struct json_object *badblocks_to_jdimms(struct
> ndctl_region *region,
> if (!found)
> goto err_found;
>
> + qsort(dimms, found, sizeof(dimm), compare_dimm_number);
> +
> for (i = 0; i < found; i++) {
> const char *devname = ndctl_dimm_get_devname(dimms[i]);
Looks good to me.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
