[ofw] [patch][opensm] Fix for win8 assert

[Uri Habusha]

We get an assert in windows when using 0, are you okay to change it to BUFSIZ in Linux and Windows code base?

-----Original Message-----
From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Hal Rosenstock
Sent: Thursday, May 26, 2011 5:30 PM
To: Fab Tillier
Cc: Galina Tcharny; OpenSM; Alex Netes; ofw at lists.openfabrics.org
Subject: Re: [ofw] [patch][opensm] Fix for win8 assert

Hi Fab,

On Thu, May 26, 2011 at 12:54 AM, Fab Tillier <ftillier at microsoft.com> wrote:
> Ideally you have a single code base for OpenSM and minimize the #ifdef sections as much as possible - that's what the patch attempted to do.

Yes that's the accepted practice: main.c is common code for Linux and Windows whereas complib and some other things are not.

> I still don't understand what a zero-size accomplishes when setting 
> _IOLBF,

From the Linux setvbuf man page:

Except  for unbuffered files, the buf argument should point to a buffer at least size bytes long; this buffer will be used instead of the  cur- rent buffer.  If the argument buf is NULL, only the mode is affected; a new buffer will be allocated on the next read or write operation.

and:

The  setbuffer()  function  is  the  same,  except that the size of the buffer is up to the caller, rather than being determined by the default BUFSIZ.  The setlinebuf() function is exactly equivalent to the call:

              setvbuf(stream, (char *)NULL, _IOLBF, 0);

so that version of the call uses BUFSIZ which is defined in stdio.h.

> it seems it should be non-zero, and probably something like 80 characters or so.

From glibc manual BUFSIZ is guaranteed to be at least 256.

> If we all agree that a non-zero size is correct in this call, then we should pick a non-zero value.  I would suggest BUFSIZ, which is the standard defined size and should be available on both Linux and Windows.

Agreed. Is the issue that 0 is not the same as BUFSIZ in Windows ? It looks to me like setvbuf in supported in Windows although line buffering may be changed into full buffering on some Windows versions.

> The 1024 seems somewhat arbitrary, not to mention rather large if the intent is to buffer a single line.
>
> That said, you could just #ifdef the call to setvbuf out for Windows like you suggest.

Yes, if we can't come up with a non ifdef solution to this.

-- Hal

> -Fab
>
> -----Original Message-----
> From: Smith, Stan [mailto:stan.smith at intel.com]
> Sent: Wednesday, May 25, 2011 5:17 PM
> To: Fab Tillier; Alex Naslednikov; ofw at lists.openfabrics.org
> Cc: Galina Tcharny; OpenSM
> Subject: RE: [ofw] [patch][opensm] Fix for win8 assert
>
> Upon further consideration, since Windows does not support line-buffered mode (flush on the occurrence of <newline> in the stream), why do we even bother with the setvbuf() call?
> Why not let Windows stdio do it's default setup and skip the setvbuf() call?
> In other words, how is ' setvbuf(stdout, NULL, _IOLBF, 1024)' advantageous in the Windows environment?
>
>
>>-----Original Message-----
>>From: Fab Tillier [mailto:ftillier at microsoft.com]
>>Sent: Wednesday, May 25, 2011 1:29 PM
>>To: Smith, Stan; Alex Naslednikov; ofw at lists.openfabrics.org
>>Cc: Galina Tcharny; OpenSM
>>Subject: RE: [ofw] [patch][opensm] Fix for win8 assert
>>
>>I tried to look up (non-MSDN) documentation for setvbuf, and I can't 
>>find anything that would indicate that _IOLBF with a size of zero 
>>would do anything other than _IONBF (line buffering with a buffer size 
>>of zero seems like not buffering, no?)
>>
>>If you want the default buffers size, why not use BUFSIZ?  That seems like it would achieve actual line buffering, using the default buffer.
>>
>>-Fab
>>
>>Smith, Stan wrote on Mon, 23 May 2011 at 11:58:05
>>
>>> Hello,
>>>   What is the logic behind a buffer size of 1024?
>>> Since this is shared code (Linux & Windows) the change should be 
>>> identified as Windows specific; as in
>>>
>>> #ifdef __WIN__
>>>      setvbuf(stdout, NULL, _IOLBF, 1024); #else
>>>      /* force stdout to be line-buffered */
>>>      setvbuf(stdout, NULL, _IOLBF, 0); #endif
>>>
>>>> -----Original Message----- From: ofw-bounces at lists.openfabrics.org
>>>> [mailto:ofw- bounces at lists.openfabrics.org] On Behalf Of Alex 
>>>> Naslednikov Sent: Sunday, May 22, 2011 4:43 AM To:
>>>> ofw at lists.openfabrics.org Cc: Galina Tcharny; OpenSM Subject: [ofw] 
>>>> [patch][opensm] Fix for win8 assert
>>>>
>>>> [opensm] fix win8 assert (2<=size<=INT_MAX): setvbuf size param 
>>>> change from 0 to 1024 Signed-off by: Galina Tcharny (galina at 
>>>> mellanox.co.il)
>>>> Index: B:/users/xalex/MLNX_VPI_trunk/ulp/opensm/user/opensm/main.c
>>>> ========================================================= 
>>>> ==========
>>>> --- B:/users/xalex/MLNX_VPI_trunk/ulp/opensm/user/opensm/main.c
>>>>     (revision 8023) +++
>>>> B:/users/xalex/MLNX_VPI_trunk/ulp/opensm/user/opensm/main.c         
>>>> (revision
>>>> 8024) @@ -692,7 +692,7 @@   };
>>>>
>>>>     /* force stdout to be line-buffered */
>>>> -   setvbuf(stdout, NULL, _IOLBF, 0);
>>>> +   setvbuf(stdout, NULL, _IOLBF, 1024);
>>>>
>>>>     /* Make sure that the opensm and complib were compiled using
>>>>        same modes (debug/free) */
>>>>
>>>> Alexander (XaleX) Naslednikov
>>>> SW Networking Team
>>>> Mellanox Technologies
>>>>
>>>>
>>>> _______________________________________________
>>>> ofw mailing list
>>>> ofw at lists.openfabrics.org
>>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>>> _______________________________________________
>>> ofw mailing list
>>> ofw at lists.openfabrics.org
>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>
_______________________________________________
ofw mailing list
ofw at lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw