Bug 195652 – [Win] Fix a slew of simple clang-cl warnings.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ross Kirsling]

[Win] Fix a slew of simple clang-cl warnings.

[Ross Kirsling]

Created attachment 364483 [details]
Patch

[Don Olmstead]

Comment on attachment 364483 [details]
Patch

r=me assuming we're green.

[EWS Watchlist]

Comment on attachment 364483 [details]
Patch

Attachment 364483 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11483638

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html

[EWS Watchlist]

Created attachment 364493 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[WebKit Commit Bot]

Comment on attachment 364483 [details]
Patch

Clearing flags on attachment: 364483

Committed r242842: <https://trac.webkit.org/changeset/242842>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48836268>

[Darin Adler]

Comment on attachment 364483 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364483&action=review

> Source/WebCore/PAL/pal/win/LoggingWin.cpp:40
> +    static const char* loggingEnvironmentVariable = "WebCoreLogging";

Should have been "const char* const"

> Source/WebCore/platform/win/LoggingWin.cpp:40
> +    static const char* loggingEnvironmentVariable = "WebCoreLogging";

Ditto.

> Source/WebKit/Platform/win/LoggingWin.cpp:41
> +    static const char* loggingEnvironmentVariable = "WebKitLogging";

Ditto.

[Ross Kirsling]

(In reply to Darin Adler from comment #8)
> > Source/WebCore/PAL/pal/win/LoggingWin.cpp:40
> > +    static const char* loggingEnvironmentVariable = "WebCoreLogging";
> 
> Should have been "const char* const"

It seemed to be a really strange typo(?) so I made it match what `auto` would give, but certainly that wouldn't stop one from overwriting the string. At that rate surely `constexpr auto` (or explicitly `constexpr const char*`) would be better though, no?

[Darin Adler]

"static const char*" is a global variable that can be overwritten with a new value. The characters can’t be overwritten, but the pointer can be.

"static const char* const" is a global constant that can’t be overwritten.

I don’t think the patch should have removed const, that was my point.

The patch changed from a constant pointer to non-constant characters (warning was about the non-constant characters pointing to read-only characters that are not safe to modify) to a *non-constant* pointer to constant characters. I don’t think making the pointer non-const was an improvement.

And yes, other ways to write it. The constexpr version sounds fine.

[Ross Kirsling]

Committed r243186: <https://trac.webkit.org/changeset/243186>