| Message ID | b369f8c90aabf121c53533ff60004b14cb19ec7b.1687943122.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: check and tune: add device and noscan options | expand |
On Wed, Jun 28, 2023 at 07:56:08PM +0800, Anand Jain wrote: > Preparatory patch adds two helper functions: array_append() and free_array(), > which facilitate reading the device list provided at the --device option. That it's for --device is for later and not that interesting when adding some API. > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > common/device-scan.c | 32 ++++++++++++++++++++++++++++++++ > common/device-scan.h | 2 ++ > 2 files changed, 34 insertions(+) > > diff --git a/common/device-scan.c b/common/device-scan.c > index 68b94ecd9d77..ba11c58d00d2 100644 > --- a/common/device-scan.c > +++ b/common/device-scan.c > @@ -31,6 +31,7 @@ > #include <dirent.h> > #include <limits.h> > #include <stdbool.h> > +#include <ctype.h> > #include <blkid/blkid.h> > #include <uuid/uuid.h> > #ifdef HAVE_LIBUDEV > @@ -540,3 +541,34 @@ int btrfs_scan_argv_devices(int dev_optind, int dev_argc, char **dev_argv) > > return 0; > } > + > +bool array_append(char **dest, char *src, int *cnt) > +{ > + char *this_tok = strtok(src, ","); > + int ret_cnt = *cnt; > + > + while(this_tok != NULL) { > + ret_cnt++; > + dest = realloc(dest, sizeof(char *) * ret_cnt); > + if (!dest) > + return false; > + > + dest[ret_cnt - 1] = strdup(this_tok); > + *cnt = ret_cnt; > + > + this_tok = strtok(NULL, ","); > + } > + > + return true; > +} > + > +void free_array(char **prt, int cnt) > +{ > + if (!prt) > + return; > + > + for (int i = 0; i < cnt; i++) > + free(prt[i]); > + > + free(prt); > +} Looks like this is an extensible pointer array, we could use that in more places where there are repeated parameters and we need to track all the values (not just the last one). Then this should be in a structure and the usage side will do only something like ptr_array_append(&array, newvalue), and not that all places will have to track the base double pointer, count and has to handle allocation failures. This should be wrapped into an API.
On Thu, Jul 13, 2023 at 08:41:01PM +0200, David Sterba wrote: > On Wed, Jun 28, 2023 at 07:56:08PM +0800, Anand Jain wrote: > > +bool array_append(char **dest, char *src, int *cnt) > > +{ > > + char *this_tok = strtok(src, ","); > > + int ret_cnt = *cnt; > > + > > + while(this_tok != NULL) { > > + ret_cnt++; > > + dest = realloc(dest, sizeof(char *) * ret_cnt); > > + if (!dest) > > + return false; > > + > > + dest[ret_cnt - 1] = strdup(this_tok); > > + *cnt = ret_cnt; > > + > > + this_tok = strtok(NULL, ","); > > + } > > + > > + return true; > > +} > > + > > +void free_array(char **prt, int cnt) > > +{ > > + if (!prt) > > + return; > > + > > + for (int i = 0; i < cnt; i++) > > + free(prt[i]); > > + > > + free(prt); > > +} > > Looks like this is an extensible pointer array, we could use that in > more places where there are repeated parameters and we need to track all > the values (not just the last one). > > Then this should be in a structure and the usage side will do only > something like ptr_array_append(&array, newvalue), and not that all > places will have to track the base double pointer, count and has to > handle allocation failures. This should be wrapped into an API. I did a quick search for reallocs, that usually mean there's an extensible array and there are too many, so I get you wanted add one more and be done. But now it looks like a proper structure should be used. I'll try to implement something.
On 15/07/2023 05:28, David Sterba wrote: > On Thu, Jul 13, 2023 at 08:41:01PM +0200, David Sterba wrote: >> On Wed, Jun 28, 2023 at 07:56:08PM +0800, Anand Jain wrote: >>> +bool array_append(char **dest, char *src, int *cnt) >>> +{ >>> + char *this_tok = strtok(src, ","); >>> + int ret_cnt = *cnt; >>> + >>> + while(this_tok != NULL) { >>> + ret_cnt++; >>> + dest = realloc(dest, sizeof(char *) * ret_cnt); >>> + if (!dest) >>> + return false; >>> + >>> + dest[ret_cnt - 1] = strdup(this_tok); >>> + *cnt = ret_cnt; >>> + >>> + this_tok = strtok(NULL, ","); >>> + } >>> + >>> + return true; >>> +} >>> + >>> +void free_array(char **prt, int cnt) >>> +{ >>> + if (!prt) >>> + return; >>> + >>> + for (int i = 0; i < cnt; i++) >>> + free(prt[i]); >>> + >>> + free(prt); >>> +} >> >> Looks like this is an extensible pointer array, we could use that in >> more places where there are repeated parameters and we need to track all >> the values (not just the last one). >> >> Then this should be in a structure and the usage side will do only >> something like ptr_array_append(&array, newvalue), and not that all >> places will have to track the base double pointer, count and has to >> handle allocation failures. This should be wrapped into an API. > > I did a quick search for reallocs, that usually mean there's an > extensible array and there are too many, so I get you wanted add one more > and be done. But now it looks like a proper structure should be used. > I'll try to implement something. Yes, indeed, this has to be an API. I thought it could be done separately. Thanks.
diff --git a/common/device-scan.c b/common/device-scan.c index 68b94ecd9d77..ba11c58d00d2 100644 --- a/common/device-scan.c +++ b/common/device-scan.c @@ -31,6 +31,7 @@ #include <dirent.h> #include <limits.h> #include <stdbool.h> +#include <ctype.h> #include <blkid/blkid.h> #include <uuid/uuid.h> #ifdef HAVE_LIBUDEV @@ -540,3 +541,34 @@ int btrfs_scan_argv_devices(int dev_optind, int dev_argc, char **dev_argv) return 0; } + +bool array_append(char **dest, char *src, int *cnt) +{ + char *this_tok = strtok(src, ","); + int ret_cnt = *cnt; + + while(this_tok != NULL) { + ret_cnt++; + dest = realloc(dest, sizeof(char *) * ret_cnt); + if (!dest) + return false; + + dest[ret_cnt - 1] = strdup(this_tok); + *cnt = ret_cnt; + + this_tok = strtok(NULL, ","); + } + + return true; +} + +void free_array(char **prt, int cnt) +{ + if (!prt) + return; + + for (int i = 0; i < cnt; i++) + free(prt[i]); + + free(prt); +} diff --git a/common/device-scan.h b/common/device-scan.h index 0d0f081134f2..d3b5f7d2753f 100644 --- a/common/device-scan.h +++ b/common/device-scan.h @@ -59,5 +59,7 @@ int add_seen_fsid(u8 *fsid, struct seen_fsid *seen_fsid_hash[], void free_seen_fsid(struct seen_fsid *seen_fsid_hash[]); int test_uuid_unique(const char *uuid_str); int btrfs_scan_argv_devices(int dev_optind, int argc, char **argv); +bool array_append(char **dest, char *src, int *cnt); +void free_array(char **prt, int cnt); #endif
Preparatory patch adds two helper functions: array_append() and free_array(), which facilitate reading the device list provided at the --device option. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- common/device-scan.c | 32 ++++++++++++++++++++++++++++++++ common/device-scan.h | 2 ++ 2 files changed, 34 insertions(+)