Sakura: TCHAR対応_stprintfが残っている

[usagisita]

問題内容

TCHAR系マクロ_stprintfが2か所残っています。
不要なら他のsprintf系関数に置換したほうがいいと思います。

特にParseCommandLineのほうは、リソース文字列表示なので、特殊な言語ファイルを読み込むと、バッファオーバーランする可能性があるので、それっぽい関数に変更してほしいです。
https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/_main/CCommandLine.cpp#L325-L329
https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/util/shell.cpp#L569

(取り敢えず、Issuesで報告します)

問題のカテゴリ

• その他の問題

[berryzplus]

概要

1240 のコメントに書いてきたんですけど、この部分の対応が未実施です。

特にParseCommandLineのほうは、リソース文字列表示なので、特殊な言語ファイルを読み込むと、バッファオーバーランする可能性があるので、それっぽい関数に変更してほしいです。

「それっぽい関数」って何やねん！

Windowsのリソース関数には、リソースの種類に応じて色んなものがあります。
ビットマップなら LoadBitmap、文字列なら LoadString というように専用関数があります。
リソースの種類に関係なく、EXE/DLLに含まれるリソースを直接検索する FindResource なんてものもありますが、「リソースの種類」に応じた専用関数を使うのが基本です。

このあたりのWindows API基礎知識を踏まえると「何かの情報を埋め込むことができる文字列」に対する「リソースの種類」があるんじゃないか？と考えられると思います。

利用できる専用関数の一覧はFindResourceEx関数のマニュアルに書いてあります。
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-findresourceexa

「何かの情報を埋め込むことができる文字列」を処理する専用関数・・・
FormatMessage関数がこれに該当すると思われます。
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage

サクラエディタの課題事項

サクラエディタのリソース文字列は、現在すべて RT_STRING になっています。
これは EXE/DLL 内に String Table を持たせ、LoadString関数によりコピーして使う形式です。

仮に FormatMessage 関数を使う場合、問題が3つあります。

• FormatMessage 関数が参照するのは String Table ではなく Message Table なのでリソースの変更が必要。
• Message Table を手で作るのはやや大変なので、mc.exeを導入することになる。
• メッセージリソースに切り出すべき文字列は、言語に応じて表現が変わる可能性のある埋込を含む文字列ですが、現在リソースに切り出されている文字列には、言語が変わっても表現が変わらないロジカルな単純埋込が含まれている。

今回修正した stprintf の出力文字列が言語に応じて変わるか？と考えると分かりやすいと思います。

• コマンドライン文字列のパターンは言語が変わっても変わらない
  
  
  
  • 本来はリソース化すべきでないと思われます。
  
  
• ヘルプサイトURLのパターンは言語が変わっても変わらない
  
  
  
  • ここはリソースでなくベタ書きになっているので問題ありません。
  
  

とりあえずの結論

たぶんまだしばらく無理。

[beru]

「それっぽい関数」って何やねん！

前の段落を見ると 不要なら他のsprintf系関数に置換 と書かれています。
つまり…ここからberryzplus さんがモダンな format 系のライブラリを作成する旅路に出るという事です。

[berryzplus]

たぶん旅立たないぞ。

現状の分析からすると、フォーマットのやり方じゃなくて「無駄にリソース化してるとこをベタ書きに戻そうぜ！」って言い出す可能性のが高いです。

理由があるならモダンじゃなくていいし、C言語でもアセンブラでもいいです。

[usagisita]

別にFormatMessageみたいな深い意味があって「それっぽい関数」と書いたわけじゃないっす。
_stprintfにはバッファ長を指定するパラメーターが無く、マクロ展開後の直接対応する関数は_swprintfのアンダーバー付きの関数がおそらく使われています。
なんかバッファあふれそうな予感がする程度の指摘でした。
それに対して、修正PRしてもらえたほうはバッファ長指定ありの標準のswprintfになっているので、とりあえずはいいんじゃないでしょうか。
ただし前の部分よく見てみると
msg_str[MAX_PATH+1]
szPath[MAX_PATH]
それにリソース文字列ぶんは最大で必要なので、msg_strはちょっと長さが足りない気がします。
ErrorMessageに置き換えればいいかもしれない、というのもあります。
似たような問題コードはたくさんありそうではありますが探すのも手間ですね。

[beru]

ただし前の部分よく見てみると
msg_str[MAX_PATH+1]
szPath[MAX_PATH]
それにリソース文字列ぶんは最大で必要なので、msg_strはちょっと長さが足りない気がします。
ErrorMessageに置き換えればいいかもしれない、というのもあります。
似たような問題コードはたくさんありそうではありますが探すのも手間ですね。

これはおっしゃる通りですね。#1283 を作成しました。

リソースから文字列を読んで printf 系の関数の書式文字列としている箇所を全て点検するのは確かに手間ですね。

プログラミングの話でいうと、こういうケースで必ずしもスタック上の固定長バッファを使う必要はないので、最大で必要になるサイズを求めてヒープに動的確保したバッファを使うやり方の方が良いですね。C++で書かれた文字列 format 処理のライブラリ内部でそういう処理を行う事が前提ですが…（色々な箇所で毎回書くのは冗長過ぎるので）。

C言語だとスタック上の固定長バッファを使うやり方で書くのがお手軽で処理も軽量なんですが、処理内容によっては不具合の要因になりますね。