Re: [mdadm PATCH 1/1] Fix a build error

[Jes Sorensen <jsorensen@fb.com>]

On 09/29/2017 09:19 PM, Xiao Ni wrote:
> 
> 
> On 09/30/2017 05:47 AM, Jes Sorensen wrote:
>> On 09/19/2017 11:48 PM, Xiao Ni wrote:
>>> On the s390 platform the build fails with the error below.
>>> Manage.c: In function 'Manage_subdevs':
>>> Manage.c:1502:5: error: passing argument 3 of 'fstat_is_blkdev' from 
>>> incompatible pointer type [-Werror]
>>>       fstat_is_blkdev(tfd, dv->devname, &rdev);
>>>       ^
>>> In file included from Manage.c:25:0:
>>> mdadm.h:1446:12: note: expected 'dev_t *' but argument is of type 
>>> 'long unsigned int *'
>>>
>>> It was introduced by commit 0a6bff09 (mdadm/util: unify fstat
>>> checking blkdev into function). It needs to pass a type 'dev_t'
>>> argument to fstat_is_blkdev, but it passes a type 'unsigned
>>> long' argument. So use a temporary variable to fix this.
>>>
>>> Signed-off-by: Xiao Ni <xni@redhat.com>
>>> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>> Suggested-by: Wols Lists <antlists@youngman.org.uk>
>>> ---
>>>   Manage.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> So having a quick look at this, I have to say I don't like the casting 
>> back and forth. The fact that we carry rdev in an unsigned long in 
>> Manage_subdevs() seems dubious to me.
>>
>> Did you look into what the implications would be to change it to a dev_t?
>>
>> Jes
> 
> Hi Jes
> 
> Do you mean define rdev as dev_t at first? It will change a lot if we do 
> so. The argument rdev is passed
> to many functions now. Such as Manage_add, Manage_remove, 
> hot_remove_disk and so on. So I think
> it's the reason why we carry rdev in an unsigned long in Manage_subddevs().
> 
> Do you have a better solution?

I looked at the other functions and they all carry rdev as a dev_t so I 
made it the same in Manage_subdevs(). We may have to clean up some more 
functions to take the right input argument, but it looks like the 
correct solution to me.

Cheers,
Jes