Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace snprintf with a C++98 equivalent #3361

Merged
merged 1 commit into from
Nov 27, 2015
Merged

replace snprintf with a C++98 equivalent #3361

merged 1 commit into from
Nov 27, 2015

Conversation

eelstork
Copy link
Contributor

snprintf is not consistently available on Windows.
Although patchy support is possible (search snprintf in suggested reference), I find that snprintf may be advantageously replaced by a solution which doesn't involve defining arbitrarily sized buffers and converting between C and C++ strings.
As I understand that C++ streams are not very popular and the main use of snprintf in Caffe is producing numbered batches, I suggest the introduction of a utility function (format_int).

@ronghanghu
Copy link
Member

IMHO snprintf is a standard io function in C/C++, available in <stdio.h> / <cstdio> and is (or at least should be) cross-platform. http://www.cplusplus.com/reference/cstdio/snprintf/

Note: Microsoft has finally implemented snprintf in Visual Studio 2015.

@eelstork
Copy link
Contributor Author

@ronghanghu specifically, snprintf is available in C99 and C++11, not C++98 (for proof, replacing <stdio.h> with <cstdio> will break C.I. builds).
VS2015 is a bit too new for most Windows build workers.
This is a compatibility issue.

@eelstork
Copy link
Contributor Author

Opinions on this PR?

@ronghanghu
Copy link
Member

This all looks good to me. I'll try to review during the Thanksgiving (got a bit overwhelmed myself these days since it is near the end of the semester).

@ronghanghu ronghanghu self-assigned this Nov 26, 2015
@eelstork
Copy link
Contributor Author

@ronghanghu thanks! Don't forget to relax during holidays.

ronghanghu added a commit that referenced this pull request Nov 27, 2015
replace snprintf with a C++98 equivalent
@ronghanghu ronghanghu merged commit c7ee261 into BVLC:master Nov 27, 2015
@ronghanghu
Copy link
Member

Thanks @eelstork !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants