On Tue, Feb 13, 2018 at 05:34:56PM +0200, Nikolay Borisov wrote: > >>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > >>> index 02c7766e6849..8112619cac95 100644 > >>> --- a/fs/btrfs/super.c > >>> +++ b/fs/btrfs/super.c > >>> @@ -346,7 +346,7 @@ static const match_table_t tokens = { > >>> {Opt_barrier, "barrier"}, > >>> {Opt_max_inline, "max_inline=%u"}, > >>> {Opt_alloc_start, "alloc_start=%s"}, > >>> - {Opt_thread_pool, "thread_pool=%d"}, > >>> + {Opt_thread_pool, "thread_pool=%u"}, > >>> {Opt_compress, "compress"}, > >>> {Opt_compress_type, "compress=%s"}, > >>> {Opt_compress_force, "compress-force"}, > >>> @@ -596,12 +596,11 @@ int btrfs_parse_options(struct btrfs_fs_info > >>> *info, char *options, > >>> ret = match_int(&args[0], &intarg); > >>> if (ret) { > >>> goto out; > >>> - } else if (intarg > 0) { > >>> - info->thread_pool_size = intarg; > >>> - } else { > >>> + } else if (intarg == 0) { > >> > >> One thing I'm worried about is the fact that match_int parses a signed > >> int. So If someone pases -1 then it would be parsed to -1 but when you > >> set it to thread_pool_size the actual value is going to be the 2's > >> complement of the value i.e. a very large number. So a check for intarg > >> < 0 is required to avoid that. Same applies to your other patches > > > > That's not true. When -o thread_pool=-1 is passed it would fail > > to match to any token. And same applies to other patches too. > > Indeed, the subtleties of the matching machinery. In that case for the > whole series:
Yeah, it's not very clear from the sources, but with %u the parser refuses negative numbers. Even with match_int, the typecasts will not damage the result when assigning to u32 from intarg. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html