gui: Drop boost::scoped_array and use wchar_t API explicitly on Windows #13734

pull ken2812221 wants to merge 2 commits into bitcoin:master from ken2812221:drop-boost-scoped-array changing 3 files +10 −28
  1. ken2812221 commented at 10:24 PM on July 21, 2018: contributor

    Drop boost::scoped_array and simplify the code.

    TCHAR should be defined as wchar_t if UNICODE is defined. So we can use .toStdWString().c_str() to get wchar_t C-style string.

    Fix #13819

  2. MarcoFalke added the label GUI on Jul 22, 2018
  3. MarcoFalke renamed this:
    Drop boost::scoped_array
    gui: Drop boost::scoped_array
    on Jul 22, 2018
  4. fanquake added this to the "In progress" column in a project
  5. fanquake commented at 4:08 AM on July 22, 2018: member

    This was introduced in #5793. Looks like something similar to what you are doing was proposed at the time as well.

  6. ken2812221 commented at 4:47 AM on July 22, 2018: contributor

    Hope there is a way to see code that is before he force push.

  7. practicalswift commented at 5:42 AM on July 22, 2018: contributor

    Concept ACK

    Thanks for removing Boost dependencies. Keep it coming! :-)

  8. MarcoFalke commented at 12:37 PM on July 22, 2018: member

    Any suggestion on how to test this?

  9. ken2812221 commented at 1:50 PM on July 22, 2018: contributor

    This is Windows-only code, can be tested as same as #5793 (comment)

  10. MarcoFalke added the label Needs gitian build on Jul 22, 2018
  11. DrahtBot removed the label Needs gitian build on Jul 22, 2018
  12. in src/qt/guiutil.cpp:617 in 06430539fb outdated 
     614 | @@ -625,7 +615,7 @@ bool SetStartOnSystemStartup(bool fAutoStart)
 615 |  #ifndef UNICODE
 616 |              psl->SetArguments(strArgs.toStdString().c_str());
 617 |  #else
    Sjors commented at 3:35 PM on July 24, 2018:

    Add a comment that UNICODE is only defined for Windows? (if that's the case)

  13. Sjors commented at 3:49 PM on July 24, 2018: member

    Should the reference also be removed from test/lint/lint-includes.sh?

    Tested that make still works on macOS.

    Tested the gitian binary on a Windows 10 VM, by launching with bitcoin-qt -wallet=你好, assuming that was the problem? This leads to a crash, but that's also the case on master:

    <img width="509" alt="schermafbeelding 2018-07-24 om 17 43 26" src="https://user-images.githubusercontent.com/10217/43150188-976eb0e8-8f69-11e8-9d97-861e005d9c7d.png">

    I have a version from earlier this year where it leads to a slightly nicer crash: <img width="494" alt="schermafbeelding 2018-07-24 om 17 46 37" src="https://user-images.githubusercontent.com/10217/43150318-e859c01a-8f69-11e8-8dca-ac36aa8774f6.png">

    (seems unrelated, so I made a ticket #13754)

    bitcoin-qt -wallet=test works fine by the way, maybe that was all.

  14. ken2812221 force-pushed on Jul 29, 2018
  15. ken2812221 commented at 11:19 AM on July 29, 2018: contributor

    Update: Since the goal is to make it unicode compatible, just make it use wchar_t version of api explicitly.

  16. ken2812221 renamed this:
    gui: Drop boost::scoped_array
    gui: Drop boost::scoped_array and use wchar_t API explicitly on Windows
    on Jul 29, 2018
  17. in src/qt/guiutil.cpp:559 in 5d70ab06a1 outdated 
     554 |  
 555 |          if (SUCCEEDED(hres))
 556 |          {
 557 |              // Get the current executable path
 558 | -            TCHAR pszExePath[MAX_PATH];
 559 | -            GetModuleFileName(nullptr, pszExePath, sizeof(pszExePath));
    ken2812221 commented at 11:25 AM on July 29, 2018:

    This is bug before, if TCHAR is wchar_t, then sizeof(pszExePath)= 2*MAX_PATH, which can result memory overflow.

  18. bitcoin deleted a comment on Jul 29, 2018
  19. MarcoFalke added the label Windows on Jul 29, 2018
  20. MarcoFalke added the label Needs gitian build on Jul 29, 2018
  21. DrahtBot removed the label Needs gitian build on Jul 29, 2018
  22. Sjors commented at 1:23 PM on July 30, 2018: member

    Tested that make still works on macOS with 5d70ab0. I have no opinion on the code itself, other than it seems good to keep making progress here: https://github.com/bitcoin/bitcoin/projects/3#card-205363

  23. MarcoFalke commented at 2:15 PM on July 31, 2018: member

    Tested that for commit 6031f7f (master and this pull) I can launch regtest and testnet after re-login for a user with only ascii character in the name.

    (See lower left of screenshot, Compare to #5793 (comment))

    screenshot from 2018-07-31 10-13-36

  24. MarcoFalke requested review from jonasschnelli on Jul 31, 2018
  25. ken2812221 force-pushed on Jul 31, 2018
  26. ken2812221 commented at 3:20 PM on July 31, 2018: contributor

    @MarcoFalke Can you test this again? Now it should fix #13819.

  27. MarcoFalke added the label Needs gitian build on Jul 31, 2018
  28. MarcoFalke added this to the milestone 0.17.0 on Jul 31, 2018
  29. DrahtBot removed the label Needs gitian build on Aug 1, 2018
  30. MarcoFalke commented at 3:36 PM on August 1, 2018: member

    Now it fails for commit d41914b (master and this pull) that it can not write to the directory:

    (though the folders are created, as can be seen in the explorer in the background)

    screenshot from 2018-08-01 11-35-25

  31. MarcoFalke removed this from the milestone 0.17.0 on Aug 1, 2018
  32. MarcoFalke added this to the milestone 0.18.0 on Aug 1, 2018
  33. ken2812221 commented at 3:52 PM on August 1, 2018: contributor

    @MarcoFalke That should fixed by #13426. You can merge these two PR and test locally.

  34. in src/qt/guiutil.cpp:556 in cefd242d60 outdated 
     555 |          if (SUCCEEDED(hres))
 556 |          {
 557 |              // Get the current executable path
 558 | -            TCHAR pszExePath[MAX_PATH];
 559 | -            GetModuleFileName(nullptr, pszExePath, sizeof(pszExePath));
 560 | +            wchar_t pszExePath[MAX_PATH];
    Empact commented at 4:22 AM on August 3, 2018:

    I think safer to use WCHAR here, as I'm seeing differing descriptions of its definition, e.g. unsigned wchar_t here: https://docs.microsoft.com/en-us/windows/desktop/intl/windows-data-types-for-strings

    ken2812221 commented at 5:16 PM on August 3, 2018:

    fixed

  35. in src/util.cpp:1121 in cefd242d60 outdated 
    1117 | @@ -1118,9 +1118,9 @@ void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length) {
1118 |  #ifdef WIN32
1119 |  fs::path GetSpecialFolderPath(int nFolder, bool fCreate)
1120 |  {
1121 | -    char pszPath[MAX_PATH] = "";
1122 | +    wchar_t pszPath[MAX_PATH] = L"";
    Empact commented at 4:23 AM on August 3, 2018:

    Same, WCHAR seems preferable.

    ken2812221 commented at 5:16 PM on August 3, 2018:

    fixed

  36. DrahtBot cross-referenced this on Aug 3, 2018 from issue utils: drop boost::interprocess::file_lock by ken2812221
  37. DrahtBot commented at 4:59 PM on August 3, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->No more conflicts as of last run.

  38. Drop boost::scoped_array 1c5d225853
  39. ken2812221 force-pushed on Aug 3, 2018
  40. in src/util.cpp:1123 in b446918fc6 outdated 
    1117 | @@ -1118,9 +1118,9 @@ void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length) {
1118 |  #ifdef WIN32
1119 |  fs::path GetSpecialFolderPath(int nFolder, bool fCreate)
1120 |  {
1121 | -    char pszPath[MAX_PATH] = "";
1122 | +    WCHAR pszPath[MAX_PATH] = L"";
1123 |  
1124 | -    if(SHGetSpecialFolderPathA(nullptr, pszPath, nFolder, fCreate))
1125 | +    if(SHGetSpecialFolderPathW(nullptr, pszPath, nFolder, fCreate))
    Empact commented at 6:37 PM on August 3, 2018:

    nit: the failure message below references SHGetSpecialFolderPathA

    ken2812221 commented at 6:48 PM on August 3, 2018:

    fixed

  41. Empact commented at 6:37 PM on August 3, 2018: member

    utACK b446918

  42. gui: get special folder in unicode bb6ca65f98
  43. ken2812221 force-pushed on Aug 3, 2018
  44. ken2812221 cross-referenced this on Aug 3, 2018 from issue Filename and command line encoding issue on Windows by ken2812221
  45. DrahtBot cross-referenced this on Aug 3, 2018 from issue Test for Windows encoding issue by ken2812221
  46. DrahtBot cross-referenced this on Aug 3, 2018 from issue [bugfix] Fix encoding issue for Windows by ken2812221
  47. ken2812221 closed this on Aug 21, 2018
  48. ken2812221 reopened this on Aug 21, 2018
  49. bitcoin deleted a comment on Aug 21, 2018
  50. bitcoin deleted a comment on Aug 21, 2018
  51. laanwj commented at 8:57 AM on September 11, 2018: member

    nice cleanup, too

    utACK bb6ca65f9890e8280ace32de5a37774e14705859

  52. laanwj merged this on Sep 11, 2018
  53. laanwj closed this on Sep 11, 2018
  54. laanwj referenced this in commit 362518791a on Sep 11, 2018
  55. ken2812221 deleted the branch on Sep 11, 2018
  56. fanquake moved this from the "In progress" to the "Done" column in a project
  57. Bushstar cross-referenced this on Oct 11, 2018 from issue Updates from bitcoin/master by Bushstar
  58. Warrows referenced this in commit 34bbaeb75f on Oct 14, 2019
  59. Warrows referenced this in commit 542a1db4de on Nov 23, 2019
  60. deadalnix referenced this in commit c61d5a151d on Apr 29, 2020
  61. ftrader referenced this in commit fb95042820 on Aug 17, 2020
  62. str4d cross-referenced this on Nov 23, 2020 from issue Backport Boost removal PRs by str4d
  63. zkbot referenced this in commit 77c2a5f810 on Dec 4, 2020
  64. Fuzzbawls cross-referenced this on Jan 30, 2021 from issue [GUI] Use wchar_t API explicitly on Windows by Fuzzbawls
  65. random-zebra referenced this in commit f15a1674ce on Feb 6, 2021
  66. furszy cross-referenced this on Jun 24, 2021 from issue Solve filename and command line encoding issues on Windows by furszy
  67. Munkybooty referenced this in commit 60ff2a65b3 on Jul 7, 2021
  68. Munkybooty referenced this in commit 6537edc78c on Jul 8, 2021
  69. Munkybooty referenced this in commit 223a358002 on Jul 9, 2021
  70. Munkybooty referenced this in commit df4af75a8c on Jul 11, 2021
  71. Munkybooty referenced this in commit d45b55dca5 on Jul 12, 2021
  72. Munkybooty referenced this in commit 3f79d489d1 on Jul 12, 2021
  73. Munkybooty referenced this in commit 57c7050f4d on Jul 12, 2021
  74. Munkybooty referenced this in commit c585c4a792 on Jul 13, 2021
  75. kwvg referenced this in commit 000fdcb6bc on Jul 13, 2021
  76. kwvg referenced this in commit 98ae047fdf on Jul 13, 2021
  77. kwvg referenced this in commit 0e863d1dee on Jul 13, 2021
  78. random-zebra referenced this in commit 61a098a775 on Aug 5, 2021
  79. bitcoin locked this on Sep 8, 2021
Contributors
ken2812221fanquakepracticalswiftMarcoFalkeSjorsEmpactDrahtBotlaanwj


jonasschnelli

Labels
GUIWindows

Milestone
0.18.0

Linked (view graph)
#5793 [Qt] honor current network when creating autostart link#13426 [bugfix] Fix encoding issue for Windows#13819 Windows: Can not create startup link if the username is a_ä_α_Б#13869 Filename and command line encoding issue on Windows
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:55 UTC