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

CPPAのリファクタリングを試行する #1815

Closed

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Mar 2, 2022

PR の目的

タイトル通りです。

カテゴリ

  • 実験 (master へのマージを目的としない)

PR の背景

CPPA::stdErrorの処理が不正っぽい、という報告が上がってます。

一連の調査で「不正ってほどマズくはない」ことが判明しているのですが、
該当コードは現状でテスト不能であり、処理内容が判読しづらいので、
ある程度「組みなおし」が必要と考えています。

指摘された問題箇所に関しての修正はできたのですが、
他部分にも影響しそうなので、CPPAに関する全体テストが必要かどうかを悩んでいます。

PR のメリット

PR のデメリット (トレードオフとかあれば)

仕様・動作説明

説明はあとで更新すると思います。

実験で確認したいのは、修正箇所に対するカバレッジがどれくらいになるか?という点です。

PR の影響範囲

テスト内容

テスト1

手順

関連 issue, PR

参考資料

ロジックの目的が読み取りづらいため。
IsBadStringPtrAは、メモリアクセス時に発生した「例外」を握りつぶす構造なので危険。
名前の印象に反して「安全にチェックできる関数」ではなく、利用すべきでない。
サイズを定義することにより、範囲外アクセスを防ぎやすくなる。
CPPA::InitDllImpのリファクタリング
@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

カバレッジ 26%で少なすぎるので「全体テストが必要」という結論になりました。

改善確認に使いたいのでもうしばらく残しておきます。

@AppVeyorBot
Copy link

マクロ機能のコア部分らしいのであちこちに影響しそう。
This reverts commit 19229b5.
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

バッチラベルが長過ぎて認識できなかった模様。。。
間違ってたので修正。
スタブに対して静的解析を実施したくないので分析除外はそのまま残す。
@AppVeyorBot
Copy link

@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 90 Code Smells

58.3% 58.3% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

sonarscan.ymlに対する変更はマージされた後じゃないと効果がないことに気付く。

@berryzplus
Copy link
Contributor Author

区切りが着いたので閉じます。

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

Successfully merging this pull request may close these issues.

2 participants