On 4/11/23 15:40, Chaitanya Kulkarni wrote: > On 4/10/23 22:29, Damien Le Moal wrote: >> On 4/11/23 02:47, Nitesh Shetty wrote: >>>> static int g_gb = 250; >>>> -module_param_named(gb, g_gb, int, 0444); >>>> +NULL_PARAM(gb, 1, INT_MAX); >>> This value gets converted to mb, for dev->size calculation in >>> null_alloc_dev. I think either there should be a type conversion or >>> this module parameter max value can be reduced to smaller value. >> Yeah, good catch. it is multiplied by 1024, and assigned to dev->size which is >> an unsigned long. So that could overflow on 32-bits arch. So this needs some fixing. >> >> I would still allow a very large value as possible though, to allow testing for >> overflows. > > will change the type in next version, but still keep the large value > possible for that type. > >>>> +device_param_cb(gb, &null_gb_param_ops, &g_gb, 0444); >>>> MODULE_PARM_DESC(gb, "Size in GB"); >> Chaitanya, >> >> Another thing: did you check if setting all these arguments through configfs >> also gets the same min/max value treatment ? Ideally, we want both configuration >> interfaces (module args and configfs) to be equivalent. > > I'm trying to avoid that in the same patch, > are you okay to add that in the same patch or a separate one ? Separate patch is fine. > >> (Note: please use dlemoal@xxxxxxxxxx. wdc.com addresses do not work right now) > > noticed that from bounced mails from hgst server, will fix it next > git-send... > > -ck > >
