search

auriamg / macdylibbundler

Utility to ease bundling libraries into executables for OSX
MIT License
550 stars 83 forks source link

off-by-one error leads to space at the end of every path #9

Closed ryandesign closed 9 years ago

ryandesign commented 9 years ago

There is an off-by-one error that causes every path processed to have a space at the end of it (mentioned in a comment in #2).

The problem is that you find the position of the string " (" in the output of otool, and then feed that to the second argument of substr, but the second argument of substr expects a length, not a position. If the start position were 0 that wouldn't be a problem, but the start position is 1 (to remove the leading tab character), so you need to substract that 1 from the found position to get the length.

Also, find finds the first occurrence of the string. What if a path for some reason contained that string " ("? Better to use rfind which finds the last occurrence of the string.

This patch fixes these problems:

https://trac.macports.org/browser/trunk/dports/devel/dylibbundler/files/patch-src-DylibBundler.cpp.diff?rev=136949

In addition, you should revert fef319a27ef2bdde6f220e3156e37b60be37609c which is not correct. fef319a27ef2bdde6f220e3156e37b60be37609c will cause problems with files whose names actually do end with a space, though that's of course very unlikely to ever occur in practice.

auriamg commented 9 years ago

Thanks for the patch, I've applied it

ryandesign commented 9 years ago

Thanks. Will you still revert fef319a27ef2bdde6f220e3156e37b60be37609c?

auriamg commented 9 years ago

Regarding fef319a I am unsure; it seems like it was originally committed with a specific purpose in mind, so reverting it may break that (even though I'm not sure why). Given that filenames ending with a space should be excessively rare, I would favor the status quo - after all, much of dylibbundler is slightly hackish anyway and will fall apart if you try hard enough

ryandesign commented 9 years ago

So let's fix the hacks!

fef319a was an incorrect attempt to fix the problem now correctly fixed in this ticket in 8eb915a / 0d8c739.

None of dylibbundler will currently work with files whose names begin or end with or even contain spaces, because it doesn't properly escape file arguments anytime it runs a system command. But if all those were fixed, which I'd like to work on next, then you'd see that fef319a will be the reason why things don't work for those hypothetical files whose names end with spaces.

Implementing a test suite would be a great way to verify the program works correctly for various inputs...

auriamg commented 9 years ago

If you feel like improving the hacks, you are more than welcome! As for myself dylibbundler fills my current needs so I have no plans to do any more significant work on it. But if someone else would like to keep working on it that would be great