[openib-general] [PATCH] osm: 'chmod' on Windows

[Yevgeny Kliteynik]

Sasha Khapyorsky wrote:
> On 18:28 Sun 26 Nov     , Yevgeny Kliteynik wrote:
>> Hal, please disregard this patch.
>>
>> Sasha, see below
>>
>> Thanks.
>>
>> Yevgeny Kliteynik wrote:
>>> Sasha Khapyorsky wrote:
>>>> On 15:22 Sun 26 Nov     , Yevgeny Kliteynik wrote:
>>>>> Fixing to match 'chmod' implementation on Windows.
>>>>> Note that on Windows 'chmod' there's no such thing 
>>>>> as user/group/all permissions - the permissions are 
>>>>> aways for 'all'.
>>>>> Don't see why this would be a problem in this case,
>>>>> but still - worth mentioning.
>>>> It is because SA dump file contains privileged information like lists
>>>> of subscribed ports.
>>> Well, since it's being dumped in the same directory as the OSM log, 
>>> I guess it gets the same protection as the OSM log does, doesn't it?
>>>
>>> -- Yevgeny
>>>  
>>>>> --
>>>>> Yevgeny
>>>>>
>>>>> Signed-off-by:  Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>>>>>  
>>>>> Index: opensm/osm_sa.c
>>>>> ===================================================================
>>>>> --- opensm/osm_sa.c	(revision 10161)
>>>>> +++ opensm/osm_sa.c	(working copy)
>>>>> @@ -70,6 +70,9 @@
>>>>>  #include <opensm/osm_multicast.h>
>>>>>  #include <opensm/osm_inform.h>
>>>>>  #include <opensm/osm_service.h>
>>>>> +#ifdef WIN32
>>>>> +#include <io.h>
>>>>> +#endif /* WIN32 */
>>>>>  
>>>>>  #define  OSM_SA_INITIAL_TID_VALUE 0xabc
>>>>>  
>>>>> @@ -569,7 +572,11 @@ opensm_dump_to_file(osm_opensm_t *p_osm,
>>>>>  		return -1;
>>>>>  	}
>>>>>  
>>>>> +#ifndef WIN32
>>>>>  	chmod(path, S_IRUSR|S_IWUSR);
>>>>> +#else
>>>>> +	_chmod(path, _S_IREAD|_S_IWRITE);
>>>>> +#endif
>>>> It would be very nice to not put such unneeded #ifdef(s) around common
>>>> sources. You can simply wrap this function in your WIN32 specific
>>>> config.h, something like:
>>>>
>>>> #define chmod(a,b) _chmod(a,b)
>>>> #define S_IRUSR _S_IREAD
>>>> #define S_IWUSR _S_IWRITE
>>>>
>>>> and this should solve this problem.
>> Or even better - I moved these defines to config.h on Windows, 
>> so no patch required here.
> 
> Great, that was exactly what I meant by "WIN32 specific config.h".

Right, missed that line :)
 
> BTW, I can count 19 another occurrences of '#ifdef WIN32' and
> '#ifdef __WIN__' under management/osm tree. I guess that not each one
> can be so trivially cleaned up as in this case, but would it be
> possible to review it and reduce number of such #ifdef(s) in common
> code?
 
Good idea.
Thanks.

-- Yevgeny

> Sasha
>