Project

General

Profile

Actions
Copy link

Bug #2299

closed

std::snprintf not found in buggy STL implementations

Added by Alex Afanasyev almost 10 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Alex Afanasyev
Category:
Build
Target version:
v0.4
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(...);

Files

snprintf.patch (1.36 KB) snprintf.patch Mathias Gibbens, 04/13/2015 11:08 AM
Actions
Copy link
 #1

Updated by Junxiao Shi almost 10 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
Actions
Copy link
 #2

Updated by Alex Afanasyev almost 10 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.

Actions
Copy link
 #3

Updated by Junxiao Shi almost 10 years ago

What about using ::snprintf everywhere?

Actions
Copy link
 #4

Updated by Alex Afanasyev almost 10 years ago

I could be ok with that

Actions
Copy link
 #5

Updated by Alex Afanasyev almost 10 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.

Actions
Copy link
 #6

Updated by Alex Afanasyev almost 10 years ago

  • Status changed from New to Rejected
Actions
Copy link
 #7

Updated by Alex Afanasyev almost 10 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.

Actions
Copy link
 #8

Updated by Davide Pesavento almost 10 years ago

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

Actions
Copy link
 #9

Updated by Alex Afanasyev almost 10 years ago

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

Will replace with C version of snprintf

Actions
Copy link
 #10

Updated by Alex Afanasyev almost 10 years ago

  • Status changed from New to In Progress
Actions
Copy link
 #11

Updated by Alex Afanasyev almost 10 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 100
Actions
Copy link
 #12

Updated by Alex Afanasyev almost 10 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.

Actions
Copy link
 #13

Updated by Alex Afanasyev almost 10 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 70 to 100
Actions
Copy link
 #14

Updated by Junxiao Shi almost 10 years ago

  • Status changed from Code review to Closed
Actions
Copy link
 #15

Updated by Mathias Gibbens over 9 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?

Actions
Copy link
 #16

Updated by Junxiao Shi over 9 years ago

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

Reopening as requested by a Mathias.

Actions
Copy link
 #17

Updated by Alex Afanasyev over 9 years ago

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

Actions
Copy link
 #18

Updated by Mathias Gibbens over 9 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.

Actions
Copy link
 #19

Updated by Alex Afanasyev over 9 years ago

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

Actions
Copy link
 #20

Updated by Mathias Gibbens over 9 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.

Actions
Copy link
 #21

Updated by Alex Afanasyev over 9 years ago

  • Status changed from Feedback to Code review
Actions
Copy link
 #22

Updated by Junxiao Shi over 9 years ago

  • Status changed from Code review to Closed
Actions
Copy link

Also available in: Atom PDF