Hi Goffredo,
On 2014/12/23 4:07, Goffredo Baroncelli wrote:
> On 12/22/2014 12:34 PM, Satoru Takeuchi wrote:
>> On 2014/12/22 3:07, Goffredo Baroncelli wrote:
>>> Change a spagetti-style code (there are some "interlaced" gotos) to
>>> a more modern style...
>>>
>>> This patch removes also some #define from utils.h, which define
>>> constants used only in cmds-filesystems.c . Instead an enum
>>> is used locally in cmds-filesystems.c .
>>
>> I'm happy if you have a regression test for this patch
>> since such kind of change often cause regression.
>
> I am impressed, this is the first time that I received this kind
> of request on btrfs ml....
It's because I'd like to contribute to Btrfs not only
by submitting patches, but also by reviewing/testing
other guy's patches to improve its quality better.
>
> In which form you want this "regression test" ? I see a
> directory tests/ under btrfs-progs; but also I saw
> reference to xfstest...
Any format is OK, for example a simple shell script.
Thanks,
Satoru
>
>
>
>>
>> Thanks,
>> Satoru
>>
>>> ---
>>> cmds-filesystem.c | 106
>>> ++++++++++++++++++++++++++++--------------------------
>>> utils.h | 3 --
>>> 2 files changed, 55 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>>> index 7eaccb9..08ddb5d 100644
>>> --- a/cmds-filesystem.c
>>> +++ b/cmds-filesystem.c
>>> @@ -40,6 +40,11 @@
>>> #include "list_sort.h"
>>> #include "disk-io.h"
>>>
>>> +enum filesystem_show_scan_method {
>>> + BTRFS_SCAN_ANY,
>>> + BTRFS_SCAN_MOUNTED,
>>> + BTRFS_SCAN_LBLKID
>>> +};
>>>
>>> /*
>>> * for btrfs fi show, we maintain a hash of fsids we've already printed.
>>> @@ -853,7 +858,7 @@ static int cmd_show(int argc, char **argv)
>>> char *search = NULL;
>>> int ret;
>>> /* default, search both kernel and udev */
>>> - int where = -1;
>>> + int where = BTRFS_SCAN_ANY;
>>> int type = 0;
>>> char mp[BTRFS_PATH_NAME_MAX + 1];
>>> char path[PATH_MAX];
>>> @@ -930,61 +935,60 @@ static int cmd_show(int argc, char **argv)
>>> uuid_unparse(fsid, uuid_buf);
>>> search = uuid_buf;
>>> type = BTRFS_ARG_UUID;
>>> - goto devs_only;
>>> + where = BTRFS_SCAN_LBLKID;
>>> }
>>> }
>>> }
>>>
>>> - if (where == BTRFS_SCAN_LBLKID)
>>> - goto devs_only;
>>> -
>>> - /* show mounted btrfs */
>>> - ret = btrfs_scan_kernel(search);
>>> - if (search && !ret) {
>>> - /* since search is found we are done */
>>> - goto out;
>>> - }
>>> -
>>> - /* shows mounted only */
>>> - if (where == BTRFS_SCAN_MOUNTED)
>>> - goto out;
>>> -
>>> -devs_only:
>>> - ret = btrfs_scan_lblkid();
>>> -
>>> - if (ret) {
>>> - fprintf(stderr, "ERROR: %d while scanning\n", ret);
>>> - return 1;
>>> - }
>>> -
>>> - found = search_umounted_fs_uuids(&all_uuids, search);
>>> - if (found < 0) {
>>> - fprintf(stderr,
>>> - "ERROR: %d while searching target device\n", ret);
>>> - return 1;
>>> - }
>>> -
>>> - /*
>>> - * The seed/sprout mapping are not detected yet,
>>> - * do mapping build for all umounted fs
>>> - */
>>> - ret = map_seed_devices(&all_uuids);
>>> - if (ret) {
>>> - fprintf(stderr,
>>> - "ERROR: %d while mapping seed devices\n", ret);
>>> - return 1;
>>> + if (where == BTRFS_SCAN_MOUNTED || where == BTRFS_SCAN_ANY) {
>>> +
>>> + /* show mounted btrfs */
>>> + ret = btrfs_scan_kernel(search);
>>> + if (search && !ret) {
>>> + /* since search is found we are done */
>>> + goto out;
>>> + }
>>> +
>>> }
>>> -
>>> - list_for_each_entry(fs_devices, &all_uuids, list)
>>> - print_one_uuid(fs_devices);
>>> -
>>> - if (search && !found)
>>> - ret = 1;
>>> -
>>> - while (!list_empty(&all_uuids)) {
>>> - fs_devices = list_entry(all_uuids.next,
>>> - struct btrfs_fs_devices, list);
>>> - free_fs_devices(fs_devices);
>>> +
>>> + if (where == BTRFS_SCAN_LBLKID || where == BTRFS_SCAN_ANY) {
>>> +
>>> + ret = btrfs_scan_lblkid();
>>> +
>>> + if (ret) {
>>> + fprintf(stderr, "ERROR: %d while scanning\n", ret);
>>> + return 1;
>>> + }
>>> +
>>> + found = search_umounted_fs_uuids(&all_uuids, search);
>>> + if (found < 0) {
>>> + fprintf(stderr,
>>> + "ERROR: %d while searching target device\n",
>>> ret);
>>> + return 1;
>>> + }
>>> +
>>> + /*
>>> + * The seed/sprout mapping are not detected yet,
>>> + * do mapping build for all umounted fs
>>> + */
>>> + ret = map_seed_devices(&all_uuids);
>>> + if (ret) {
>>> + fprintf(stderr,
>>> + "ERROR: %d while mapping seed devices\n", ret);
>>> + return 1;
>>> + }
>>> +
>>> + list_for_each_entry(fs_devices, &all_uuids, list)
>>> + print_one_uuid(fs_devices);
>>> +
>>> + if (search && !found)
>>> + ret = 1;
>>> +
>>> + while (!list_empty(&all_uuids)) {
>>> + fs_devices = list_entry(all_uuids.next,
>>> + struct btrfs_fs_devices, list);
>>> + free_fs_devices(fs_devices);
>>> + }
>>> }
>>> out:
>>> printf("%s\n", BTRFS_BUILD_VERSION);
>>> diff --git a/utils.h b/utils.h
>>> index 0464c2d..603cdfb 100644
>>> --- a/utils.h
>>> +++ b/utils.h
>>> @@ -26,9 +26,6 @@
>>> #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
>>> #define BTRFS_MKFS_SMALL_VOLUME_SIZE (1024 * 1024 * 1024)
>>>
>>> -#define BTRFS_SCAN_MOUNTED (1ULL << 0)
>>> -#define BTRFS_SCAN_LBLKID (1ULL << 1)
>>> -
>>> #define BTRFS_UPDATE_KERNEL 1
>>>
>>> #define BTRFS_ARG_UNKNOWN 0
>>>
>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
