[PATCH v2] fdisk: add the 'i'nfo command

[PATCH v2] fdisk: add the 'i'nfo command

[Jean-Loup 'clippix' Bogalho]

Add the 'i'nfo command to fdisk that prints details about a specific partition
(as of now: name / offset / size / uuid / type / C/H/S).

The output format is as follow:

    name:       <name>
    start:      <offset>
    size:       <size>
    uuid:       <uuid>
    type:       <type>
    C/H/S:      <cylinders/1/sectors>

Signed-off-by: Jean-Loup 'clippix' Bogalho <clippix@lse.epita.fr>
---

This is a second version of the patch that adds the 'i'nfo command, modified
according to the review of Karel Zak. I'm still getting a NULL pointer when
asking for the device's name or uuid. Moreover, whether you ask for cylinders
or sectors, fdisk_partition_to_string always return the partition size, is it
supposed to be the wanted behavior?

 disk-utils/fdisk-menu.c |  4 ++++
 disk-utils/fdisk.c      | 43 +++++++++++++++++++++++++++++++++++++++++++
 disk-utils/fdisk.h      |  2 ++
 3 files changed, 49 insertions(+)

diff --git a/disk-utils/fdisk-menu.c b/disk-utils/fdisk-menu.c
index a77a13f..29af2c9 100644
--- a/disk-utils/fdisk-menu.c
+++ b/disk-utils/fdisk-menu.c
@@ -99,6 +99,7 @@ struct menu menu_generic = {
 		MENU_BENT ('p', N_("print the partition table")),
 		MENU_ENT  ('t', N_("change a partition type")),
 		MENU_BENT_E('v', N_("verify the partition table"), FDISK_DISKLABEL_BSD),
+		MENU_ENT  ('i', N_("print informations about a partition")),
 
 		MENU_XENT('d', N_("print the raw data of the first sector from the device")),
 		MENU_XENT('D', N_("print the raw data of the disklabel from the device")),
@@ -557,6 +558,9 @@ static int generic_menu_cb(struct fdisk_context **cxt0,
 	case 'v':
 		rc = fdisk_verify_disklabel(cxt);
 		break;
+	case 'i':
+		rc = print_partition_infos(cxt);
+		break;
 	}
 
 	/* expert mode */
diff --git a/disk-utils/fdisk.c b/disk-utils/fdisk.c
index a1e7259..b1c6d0f 100644
--- a/disk-utils/fdisk.c
+++ b/disk-utils/fdisk.c
@@ -564,6 +564,49 @@ void change_partition_type(struct fdisk_context *cxt)
 	fdisk_unref_partition(pa);
 }
 
+#define NB_INFO_FIELDS	6
+
+int print_partition_infos(struct fdisk_context *cxt)
+{
+	struct fdisk_partition *pa = NULL;
+	char *data = NULL;
+	int rc = 0;
+	size_t i;
+	static int info_ids[NB_INFO_FIELDS] = {
+		FDISK_FIELD_NAME, FDISK_FIELD_START, FDISK_FIELD_SIZE,
+		FDISK_FIELD_UUID, FDISK_FIELD_TYPE, FDISK_FIELD_CYLINDERS
+	};
+	static char *fields[NB_INFO_FIELDS] = {
+		"name", "start", "size", "uuid", "type", "C/H/S"
+	};
+
+	if ((rc = fdisk_ask_partnum(cxt, &i, FALSE)))
+		return rc;
+
+	if ((rc = fdisk_get_partition(cxt, i, &pa))) {
+		fdisk_warnx(cxt, _("Partition %zu does not exist yet!"), i + 1);
+		return rc;
+	}
+
+	for (i = 0; i < NB_INFO_FIELDS - 1; ++i) {
+		rc = fdisk_partition_to_string(pa, cxt, info_ids[i], &data);
+		if (rc < 0)
+			goto clean_data;
+		fdisk_info(cxt, _("%s:\t%s"), fields[i], data);
+		free(data);
+	}
+	if ((rc = fdisk_partition_to_string(pa, cxt, info_ids[i], &data)) < 0)
+		goto clean_data;
+	fdisk_info(cxt, _("%s:\t%s/%d/%s"), fields[i], data, 1, data);
+
+clean_data:
+	fdisk_unref_partition(pa);
+	free(data);
+	return rc;
+}
+
+#undef NB_INFO_FIELDS
+
 static size_t skip_empty(const unsigned char *buf, size_t i, size_t sz)
 {
 	size_t next;
diff --git a/disk-utils/fdisk.h b/disk-utils/fdisk.h
index 4bd4733..aadd802 100644
--- a/disk-utils/fdisk.h
+++ b/disk-utils/fdisk.h
@@ -36,6 +36,8 @@ extern int process_fdisk_menu(struct fdisk_context **cxt);
 extern int ask_callback(struct fdisk_context *cxt, struct fdisk_ask *ask,
 		    void *data __attribute__((__unused__)));
 
+extern int print_partition_infos(struct fdisk_context *cxt);
+
 /* prototypes for fdisk.c */
 extern void dump_firstsector(struct fdisk_context *cxt);
 extern void dump_disklabel(struct fdisk_context *cxt);

-- 
Jean-Loup 'clippix' Bogalho
LSE 2016

Re: [PATCH v2] fdisk: add the 'i'nfo command

[Benno Schulenberg]

On Wed, May 6, 2015, at 16:21, Jean-Loup 'clippix' Bogalho wrote:
> +		MENU_ENT  ('i', N_("print informations about a partition")),

s/informations/information/
In unjoking English it doesn't have a plural.

Benno

-- 
http://www.fastmail.com - Same, same, but different...

Re: [PATCH v2] fdisk: add the 'i'nfo command

[Jean-Loup 'clippix' Bogalho]

On Wed, May 06, 2015 at 05:45:02PM +0200, Benno Schulenberg wrote:
> 
> On Wed, May 6, 2015, at 16:21, Jean-Loup 'clippix' Bogalho wrote:
> > +		MENU_ENT  ('i', N_("print informations about a partition")),
> 
> s/informations/information/
> In unjoking English it doesn't have a plural.

Will do, thanks,

-- 
Jean-Loup 'clippix' Bogalho
LSE 2016

Re: [PATCH v2] fdisk: add the 'i'nfo command

[Karel Zak]

On Wed, May 06, 2015 at 04:21:43PM +0200, Jean-Loup 'clippix' Bogalho wrote:
> +#define NB_INFO_FIELDS	6

This is bad manner (as it makes code fragile), the better solution is:

    int foo[] = { 10, 9, 8 };

and then

    for (i = 0; i < ARRAY_SIZE(foo); i++) {
       ...
    }

> +int print_partition_infos(struct fdisk_context *cxt)
> +{
> +	struct fdisk_partition *pa = NULL;
> +	char *data = NULL;
> +	int rc = 0;
> +	size_t i;
> +	static int info_ids[NB_INFO_FIELDS] = {
> +		FDISK_FIELD_NAME, FDISK_FIELD_START, FDISK_FIELD_SIZE,
> +		FDISK_FIELD_UUID, FDISK_FIELD_TYPE, FDISK_FIELD_CYLINDERS
> +	};

 FDISK_FIELD_CYLINDERS means size in cylinders, IMHO it's unnecessary

 FDISK_FIELD_SADDR is start address and FDISK_FIELD_EADDR is end
 address in CHS notation

 Anyway, you don't have to hardcode the IDs, see function

   fdisk_label_get_fields_ids()

 it returns (allocated) array with the IDs, you need something like:

   size_t nfields;
   int *fields = NULL;
   int details;
   struct fdisk_label *lb = fdisk_get_label(cxt);

   details = fdisk_is_details(cxt);
   fdisk_enable_details(cxt, 1);
   fdisk_label_get_fields_ids(lb, cxt, &fields, &nfields);
   fdisk_enable_details(cxt, details);

 for more see code in disk-utils/fdisk-list.c

> +	static char *fields[NB_INFO_FIELDS] = {
> +		"name", "start", "size", "uuid", "type", "C/H/S"
> +	};

 This is unnecessary, libfdisk already contains the names of the
 fields. See below.

> +
> +	if ((rc = fdisk_ask_partnum(cxt, &i, FALSE)))
> +		return rc;
> +
> +	if ((rc = fdisk_get_partition(cxt, i, &pa))) {
> +		fdisk_warnx(cxt, _("Partition %zu does not exist yet!"), i + 1);
> +		return rc;
> +	}
> +	for (i = 0; i < NB_INFO_FIELDS - 1; ++i) {

        int id = info_ids[i];
        struct fdisk_field *field = fdisk_label_get_field(lb, id);

        if (!field)
            continue;   /* not supported by the current label */

> +		rc = fdisk_partition_to_string(pa, cxt, info_ids[i], &data);
> +		if (rc < 0)
> +			goto clean_data;
> +		fdisk_info(cxt, _("%s:\t%s"), fields[i], data);

        fdisk_info(cxt, "%15s: %s\n", fdisk_field_get_name(fd), data);

> +		free(data);
> +	}
> +	if ((rc = fdisk_partition_to_string(pa, cxt, info_ids[i], &data)) < 0)
> +		goto clean_data;
> +	fdisk_info(cxt, _("%s:\t%s/%d/%s"), fields[i], data, 1, data);

 _SADDR and _EADDR returns complete CHS, you don't have to compose it here.

 The stuff in the for() should be enough.

> +
> +clean_data:
> +	fdisk_unref_partition(pa);
> +	free(data);
> +	return rc;
> +}

 Thanks, the next version will be perfect :-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com