You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There are possible uses of fs::path::string() which may result in a bug.
Changing those would be desired.
Please apply the patch to the release branch release1.2.
One question: There are other instances of fs::path::string() in the code (see a list below). Should some of those also be changed? Some are for error messages, so they should not be converted to utf8 (I think), but some are really path manipulations, and should probably be converted as well.
List of fs::path::string() occurrences
src/util/PathUtil.cpp:
79: dangerous to use, but only one usage where a system encoded path is required ⇒ no bug here.
85: function returns a boolean and works over ASCII only characters. It would only be a problem if windows still uses code pages, which do not extend ASCII.
90: same as 85.
95: dangerous, no known fix, since it's ambiguous, whether const std::string& ext is UTF-8 or ANSI/System encoded. Safe when used with ASCII characters only.
297: g_warning does not know anything about encodings, it's forwarded to the console, which has most likely system encoding ⇒ string is the better choice here.
src/core/control/jobs/AutosaveJob.cpp:45 : the same of g_warning applies for g_message
src/core/control/latex/LatexGenerator.cpp:53 : The variable name already states, that the path must be encoded as OS encoded string ⇒ string is correct here, u8string would be a bug.
src/core/control/settings/MetadataManager.cpp
30: string is better choice
36: same
95: result is used as parameter to strtoll, I bet it only supports ASCII. Chinese numbers will never be accepted, whether it is utf8 or ASCII/ANSY/any other encoding.
src/core/control/settings/Settings.cpp: All are used in a g_xxxxx log function, so string is the better choice.
src/core/control/tools/ImageHandler.cpp:69 : Not part of 1.2.x, but u8string would be better here. Has only visual effects
src/core/control/xojfile/SaveHandler.cpp: Can't change those without breaking old files, u8string is better here!. But it has no negative impact on computers with the same system encoding
src/core/control/xojfile/LoadHandler.cpp:
All uses of error(...) are prone to bugs and may not work as expected.
This should be refactored. Can't tell, whether error is used only in g_xxx log functions or if it is also used in an error message.
No negative impact for the correctness of xournalpp only look and feel.
Changing string to u8string for the rest will also break old files.
src/core/control/XournalMain.cpp:
124: Safe
125: Safe
167: Safe, since ASCII only characters are used
191: u8string should be used for better look and feel, no breaking bug
446: u8string should be used for better look and feel, no breaking bug
504: Safe
src/core/control/Control.cpp:
238: u8string should be used for better look and feel, no breaking bug
1424: Safe
1608: u8string is the better choice for LAF
1609 neither string nor u8string are good here, string is required for correctness, u8string for LAF, best would be to keep filename as path.
1616: ???
1617: u8string LAF
1620: u8string LAF
1621: Same as 1609
1639: u8string LAF
src/core/gui/GladeSearchpath.cpp:37: Safe
src/core/plugin/luapi_application.h: I think lua works on the system path, but @rolandlo is the better one to ask.
src/core/plugin/Plugin.cpp:
src/core/plugin/PluginController.cpp: I think lua works on the system path, but @rolandlo is the better one to ask.
src/exe/osx/setup-env.cpp:
Completely secure, since osx and unix work on utf-8 anyway, there is no difference between u8string and string.
There are possible uses of
fs::path::string()
which may result in a bug.Changing those would be desired.
Please apply the patch to the release branch release1.2.
src/util/PathUtil.cpp:
79: dangerous to use, but only one usage where a system encoded path is required ⇒ no bug here.
85: function returns a boolean and works over ASCII only characters. It would only be a problem if windows still uses code pages, which do not extend ASCII.
90: same as 85.
95: dangerous, no known fix, since it's ambiguous, whether
const std::string& ext
is UTF-8 or ANSI/System encoded. Safe when used with ASCII characters only.297: g_warning does not know anything about encodings, it's forwarded to the console, which has most likely system encoding ⇒ string is the better choice here.
src/core/control/jobs/AutosaveJob.cpp:45 : the same of g_warning applies for g_message
src/core/control/latex/LatexGenerator.cpp:53 : The variable name already states, that the path must be encoded as OS encoded string ⇒ string is correct here,
u8string
would be a bug.src/core/control/settings/MetadataManager.cpp
30: string is better choice
36: same
95: result is used as parameter to strtoll, I bet it only supports ASCII. Chinese numbers will never be accepted, whether it is utf8 or ASCII/ANSY/any other encoding.
src/core/control/settings/Settings.cpp: All are used in a g_xxxxx log function, so string is the better choice.
src/core/control/tools/ImageHandler.cpp:69 : Not part of 1.2.x, but u8string would be better here. Has only visual effects
src/core/control/xojfile/SaveHandler.cpp: Can't change those without breaking old files, u8string is better here!. But it has no negative impact on computers with the same system encoding
src/core/control/xojfile/LoadHandler.cpp:
All uses of error(...) are prone to bugs and may not work as expected.
This should be refactored. Can't tell, whether error is used only in g_xxx log functions or if it is also used in an error message.
No negative impact for the correctness of xournalpp only look and feel.
Changing string to u8string for the rest will also break old files.
src/core/control/CrashHandler.cpp: Safe
src/core/control/LatexController.cpp: Safe
src/core/control/XournalMain.cpp:
124: Safe
125: Safe
167: Safe, since ASCII only characters are used
191: u8string should be used for better look and feel, no breaking bug
446: u8string should be used for better look and feel, no breaking bug
504: Safe
src/core/control/Control.cpp:
238: u8string should be used for better look and feel, no breaking bug
1424: Safe
1608: u8string is the better choice for LAF
1609 neither string nor u8string are good here, string is required for correctness, u8string for LAF, best would be to keep filename as path.
1616: ???
1617: u8string LAF
1620: u8string LAF
1621: Same as 1609
1639: u8string LAF
src/core/gui/GladeSearchpath.cpp:37: Safe
src/core/plugin/luapi_application.h: I think lua works on the system path, but @rolandlo is the better one to ask.
src/core/plugin/Plugin.cpp:
src/core/plugin/PluginController.cpp: I think lua works on the system path, but @rolandlo is the better one to ask.
src/exe/osx/setup-env.cpp:
Completely secure, since osx and unix work on utf-8 anyway, there is no difference between u8string and string.
Originally posted by @Febbe in #5638 (comment)
The text was updated successfully, but these errors were encountered: