Re: [PATCH 04/12] mkfs: merge tables for opts parsing into one table

Luis R. Rodriguez
2017-04-26T01:45:33Z

On Wed, Apr 26, 2017 at 03:38:15AM +0200, Luis R. Rodriguez wrote:
> On Tue, Apr 25, 2017 at 08:28:23AM -0500, Eric Sandeen wrote:
> > On 4/25/17 2:45 AM, Jan Tulak wrote:
> > > On Tue, Apr 25, 2017 at 5:04 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > >> On Sun, Apr 23, 2017 at 08:54:55PM +0200, Jan Tulak wrote:
> > >>> Merge separate instances of opt_params into one indexable table. Git
> > >>> makes this patch looks a bit more complicated, but it does not change
> > >>> values or structure of anything else. It only moves all the "struct
> > >>> opt_params dopts = {...}", changes indentation for these substructures
> > >>> and replaces their usage (dopts -> opts[OPT_D]).
> > >>>
> > >>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> > >>> ---
> > >>> mkfs/xfs_mkfs.c | 1316 +++++++++++++++++++++++++++++--------------------------
> > >>> 1 file changed, 683 insertions(+), 633 deletions(-)
> > >>>
> > >>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > >>> index 7a72b11..513e106 100644
> > >>> --- a/mkfs/xfs_mkfs.c
> > >>> +++ b/mkfs/xfs_mkfs.c
> > >>> @@ -42,6 +42,7 @@ static int ispow2(unsigned int i);
> > >>> uint64_t blocksize;
> > >>> uint64_t sectorsize;
> > >>>
> > >>> +#define MAX_OPTS 16
> > >>
> > >> This is fragile, every time a new opt is added this needs to be updated
> >
> > <pedantic>There are only 8 now, so there are still 8 free slots</pedantic>
> >
> > >> and so is the index, and we should be pedantic over not going out of bounds.
> > >> We could instead use a flexible array and compute the max opts at run time
> > >> as a global. This way the max opts is always updated automatically.
> >
> > I don't think it's all that fragile;
>
> We can avoid such compilation warnings.

Oh and save memory by 8 struct opt_params. Why not.

Luis