Project

General

Profile

Bug #2299

std::snprintf not found in buggy STL implementations

Added by Alex Afanasyev almost 5 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Category:
Build
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Description

At least with cygwin, does not properly define std::snprintf, but C's version ::snprintf is still available.

I'm proposing changing instances of std::snprintf with the following:

// inside function body
using namespace std;
snprintf(...);
snprintf.patch (1.36 KB) snprintf.patch Mathias Gibbens, 04/13/2015 11:08 AM

History

#1 Updated by Junxiao Shi almost 5 years ago

I disagree with the proposed solution. using namespace should be avoided.

  1. check for std::snprintf during configure step
  2. in common.hpp, declare std::snprintf as a function that forwards its arguments to ::snprintf

#2 Updated by Alex Afanasyev almost 5 years ago

This technique is used in many places in boost libraries and I don't see problem with that.

We can check during configure stage, but then it is just more complicated to use, while there is a simple solution that should solve all the problems.

#3 Updated by Junxiao Shi almost 5 years ago

What about using ::snprintf everywhere?

#4 Updated by Alex Afanasyev almost 5 years ago

I could be ok with that

#5 Updated by Alex Afanasyev almost 5 years ago

I decided to reject this task, as it is just a really buggy cygwin implementation. Code can be compiled with addition of a few defines.

#6 Updated by Alex Afanasyev almost 5 years ago

  • Status changed from New to Rejected

#7 Updated by Alex Afanasyev almost 5 years ago

  • Status changed from Rejected to New

I'm reopening this, as android compilation has exactly the same problem, and unfortunately I cannot fix it with a few extra defines.

#8 Updated by Davide Pesavento almost 5 years ago

Just use the C version, as it was before we moved to C++11.

#9 Updated by Alex Afanasyev almost 5 years ago

  • Assignee set to Alex Afanasyev
  • Target version set to v0.3

Will replace with C version of snprintf

#10 Updated by Alex Afanasyev over 4 years ago

  • Status changed from New to In Progress

#11 Updated by Alex Afanasyev over 4 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 100

#12 Updated by Alex Afanasyev over 4 years ago

  • Status changed from Code review to In Progress
  • % Done changed from 100 to 70

Similar fix needs to be done in NFD codebase. Will submit new patch later.

#13 Updated by Alex Afanasyev over 4 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 70 to 100

#14 Updated by Junxiao Shi over 4 years ago

  • Status changed from Code review to Closed

#15 Updated by Mathias Gibbens over 4 years ago

I hit this problem today using OpenWRT's build toolchain (gcc 4.8.3) when cross-compiling NFD 0.3.1. There is a usage of std::snprintf in ./daemon/face/ethernet-face.cpp that is causing the same problem. Alex's previous patch only fixed the issue in ./core/logger.cpp. There are no other occurrences of snprintf in the NFD 0.3.1 code.

Can we get a fix for this included in the next NFD release?

#16 Updated by Junxiao Shi over 4 years ago

  • Status changed from Closed to Feedback
  • Target version changed from v0.3 to v0.4

Reopening as requested by a Mathias.

#17 Updated by Alex Afanasyev over 4 years ago

@Mathias Can you submit your patch for the code review?

#18 Updated by Mathias Gibbens over 4 years ago

Here is a patch against the current NFD master branch that fixes the issue. It is modeled after Alex's previous two patches.

#19 Updated by Alex Afanasyev over 4 years ago

@Mathias, can you please push your patchset to http://gerrit.named-data.net/?

#20 Updated by Mathias Gibbens over 4 years ago

@Alex, I don't have an account setup for the project's Gerrit. This is such a small patch that I'd like to not go through the trouble of setting up another account just to commit it. I'd be fine with either you or Junxiao committing it.

#21 Updated by Alex Afanasyev over 4 years ago

  • Status changed from Feedback to Code review

#22 Updated by Junxiao Shi over 4 years ago

  • Status changed from Code review to Closed

Also available in: Atom PDF