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

Return std::string / C++ implementation #9

Open
Flamefire opened this issue Jul 3, 2017 · 6 comments
Open

Return std::string / C++ implementation #9

Flamefire opened this issue Jul 3, 2017 · 6 comments

Comments

@Flamefire
Copy link

The current implementation works with raw pointers. How about porting this to C++ returning a std::string instead (at least as a wrapper) for easier and less error-prone usage?

@gpakosz
Copy link
Owner

gpakosz commented Jul 3, 2017

See the wip-cpp branch.

I don't believe it makes it easier or less error-prone though.

@Flamefire
Copy link
Author

Well I'd make it a lot simpler: Just std::string getExecutablePath() Everything else is (IMO) beyond the scope of this app. No need for basename etc.
Also note that std::string[0] should not be used for writing prior to C++11. Use a vector instead and copy.

Easier because: 1 simple call to getExecutablePath() instead of all the get size, alloc, read, free steps which may result in memory leaks.

@gpakosz
Copy link
Owner

gpakosz commented Jul 3, 2017

Well I'd make it a lot simpler: Just std::string getExecutablePath() Everything else is (IMO) beyond the scope of this app. No need for basename etc.

This is just your opinion.

Also note that std::string[0] should not be used for writing prior to C++11. Use a vector instead and copy.

I don't understand why you're mentioning std::string[0], did you have a look at the wip-cpp branch?

Easier because: 1 simple call to getExecutablePath() instead of all the get size, alloc, read, free steps which may result in memory leaks.

There are many situations where people want to have control of memory allocations and are willing to do so. They won't make mistakes.

@gpakosz
Copy link
Owner

gpakosz commented Jul 3, 2017

I don't understand why you're mentioning std::string[0], did you have a look at the wip-cpp branch?

If you're thinking about the standard not mandating std::string's storage to be contiguous prior to C++11, well I've yet to see an STL implementation that doesn't use contiguous storage for std::string.

@Flamefire
Copy link
Author

Flamefire commented Jul 3, 2017

This is just your opinion.

Yes, therefore the "IMO". Reasoning would be that there are many libraries that can do path handling already (boost, C++17, ...) so I would not include it in a library whose purpose is getting the executable path. Of course it might be convenient for users not wanting to use an extra library, no question there.

I don't understand why you're mentioning std::string[0], did you have a look at the wip-cpp branch?

Yes:

getModulePath(&path[0], length, &dirname_length);

and yes I also agree that the STL implementations all use contiguous storage. But I heard, that some may have used linked lists, extra copy on access, shared pointers etc. and there is lots of discussion on whether using &std::string[0] is safe (prior C++11) so I wanted to mention it. Just "what-if" someone comes along an old, proprietary library that tried to be very clever?

Ok, explicit memory control is a good reason. But often you don't need that, that's why I was suggesting the C++ interface. Did not see the wip branch, so thanks for pointing that out.

@gpakosz
Copy link
Owner

gpakosz commented Jul 3, 2017

Reasoning would be that there are many libraries that can do path handling already (boost, C++17, ...) so I would not include it in a library whose purpose is getting the executable path.

The C++17 filesystem library is well, available from C++17 only. And AFAIK it comes from Boost which is seen as overly bloated and over-engineered by a non negligible fraction of C++ developers. Given that, it was a little effort to add a way to report the dirname part of the executable or module path.

Just "what-if" someone comes along an old, proprietary library that tried to be very clever?

Well, maybe such an STL exists in the wild. I've never seen one in production so far. Hence the choice.

I started the wip-cpp branch out of curiosity and at that point, based on feedback I collected, I still believe it brings very little value to the table.

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

No branches or pull requests

2 participants