search

MapServer / MapServer-import

3 stars 2 forks source link

msIO_printf() silently does nothing when output is larger than workbuf #2214

Closed tbonfort closed 12 years ago

tbonfort commented 12 years ago

Reporter: dmorissette Date: 2007/08/08 - 16:13 While looking into ticket #1946, I found that msIO_printf() does not print anything and just silently returns -1 when the output is larger than the allocated workBuf[8000](i.e. if vsnprintf returns a value larger than 8000).

Since calling code rarely checks the return value of printf calls, the user is never notified of the error. What would you think of adding a msSetError() call in this case? Even if the error is not trapped, it would show up in MS_ERRORFILE if it is enabled.

tbonfort commented 12 years ago

Author: dmorissette Date: 2007/08/08 - 16:13 I'll take this ticket but am interested in feedback from Frank and Steve.

tbonfort commented 12 years ago

Author: warmerdam Date: 2007/08/08 - 16:20

Daniel,

The CPLString::Printf() implementation shows a pattern for trying again on failure with a larger buffer. Be careful about the va_copy() stuff as if you get it wrong it will still work on some systems but not others.

http://trac.osgeo.org/gdal/browser/trunk/gdal/port/cplstring.cpp#L84

tbonfort commented 12 years ago

Author: sdlime Date: 2007/08/08 - 16:24 The effects of "silent" and very subtle bugs so I think any output that might help diagnose the problem. Are you proposing fixing the problem as Frank suggests or just writing to the error stack?

Steve

tbonfort commented 12 years ago

Author: dmorissette Date: 2007/08/08 - 19:44 For 5.0 I have played safe and only added a msSetError() call before returning -1 in case of buffer overrun. That's in 8ef3295a3735021a40efcd0a105791ecb245282d (r6507). The error will be visible in the logs if MS_ERRORFILE is set.

We'll keep the real fix based on GDAL's CPLString::vPrintf() for v5.2

tbonfort commented 12 years ago

Author: dmorissette Date: 2007/08/08 - 19:56 Closing this ticket as fixed since the original issue was that msIO_printf() and family were silently returning -1 and now they call msSetError().

I have created ticket #2214 about rewriting the functions (in 5.2) to automatically try again with a larger buffer like CPLString::Printf() does.