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

[Hal Rosenstock]

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
>