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

Get rid of fc/io/fstream.hpp #2047

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

pmconrad
Copy link
Contributor

@pmconrad pmconrad commented Nov 7, 2019

Part of #1116 / #1584

@abitmore abitmore added this to the 4.1.0 - Feature Release milestone Nov 8, 2019
@abitmore abitmore added this to In development in Protocol Upgrade Release (5.0.0) via automation Nov 8, 2019
#include <string>

namespace graphene { namespace utilities {

/**
* @brief Reads the file at the given path and returns the full contents in a string. Note that the result
* will contain non-printable characters if the file does.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I saw "non-printable characters", '\0' didn't come to my mind. I don't know whether it's only me though. Perhaps better if we explicitly mention '\0'? Callers need to be careful when using this function, since functions like c_str() are heavily used in the code base, a strcpy() or strlen() call on the result of c_str() may return unexpected results.

@abitmore abitmore added this to In development in Feature Release (5.1.0) via automation Sep 13, 2020
@abitmore abitmore removed this from In development in Protocol Upgrade Release (5.0.0) Sep 13, 2020
@abitmore abitmore linked an issue Sep 13, 2020 that may be closed by this pull request
17 tasks
@abitmore abitmore removed this from In development in Feature Release (5.1.0) Jan 6, 2021
@abitmore abitmore added this to In development in Feature Release (5.2.0) via automation Jan 6, 2021
@abitmore abitmore added this to In progress in Feature Release (6.1.0) via automation Apr 13, 2021
@abitmore abitmore removed this from In development in Feature Release (5.2.0) Apr 13, 2021
@abitmore abitmore removed this from In progress in Feature Release (6.1.0) Jul 31, 2022
@abitmore abitmore added this to In development in Feature Release (7.1.0) via automation Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Refactor fc::fstream to be more like std::fstream
2 participants