[wpe-2.38] Fix public/private format strings in logs

[Scony]

This PR removes hardcoded conversion strings {public} and {private}from format strings.

The current situation leads to compilation warnings like:

../git/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1730:9: note: in expansion of macro 'LOCAL_LOG'
 1730 |         LOCAL_LOG(R"(    "commentURL": "%{public}s")", escapedCommentURL.utf8().data());
      |         ^~~~~~~~~
WTF/Headers/wtf/Assertions.h:621:25: warning: unknown conversion type character '{' in format [-Wformat=]
--
      |     ^~~~~~~~~~~~~~~~~~~~
../git/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1731:9: note: in expansion of macro 'LOCAL_LOG'
 1731 |         LOCAL_LOG(R"(  }%{public}s)", trailingComma);
      |         ^~~~~~~~~
In file included from DerivedSources/WebKit/unified-sources/UnifiedSource-72468c22-3.cpp:1:
../git/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1731:26: note: format string is defined here
 1731 |         LOCAL_LOG(R"(  }%{public}s)", trailingComma);

as well as runtime issues where logs are printed as e.g.:

[WPEWebKit:Layout:4] 0xacffc870 - [webFrame=0xacff7940, webFrameID=3, webPage=0xacf9b000, webPageID=9] WebFrameLoaderClient::dispatchDidReachLayoutMilestone: dispatching DidReachLayoutMilestone (milestones=%{public}s)

while they should look like:

[WPEWebKit:Layout:4] 0xacffc870 - [webFrame=0xacff7900, webFrameID=3, webPage=0xacf9b000, webPageID=9] WebFrameLoaderClient::dispatchDidReachLayoutMilestone: dispatching DidReachLayoutMilestone (milestones=DidFirstLayout, DidFirstVisuallyNonEmptyLayout)

I've tested this change briefly on arm32 and x86_64 devices and it looks to work well. It would be nice, however, if someone with good expertise in WebKit logging could take a look.

[magomez]

@Scony, I'm trying to reproduce those warnings during the build but they are not showing for me. Which log options are you passing to the build?

[magomez]

I've been able to reproduce the problem, and after applying your patch I'm still getting warnings of this type:

WTF/Headers/wtf/Assertions.h:621:25: warning: unknown conversion type character '"' in format [-Wformat=]
  621 |         fprintf(stderr, "[%s:%s:%i] " fmt "\n", logChannel.subsystem, logChannel.name, priority, ##__VA_ARGS__); \
      |                         ^~~~~~~~~~~~~~~~~~~~~~
WTF/Headers/wtf/Assertions.h:624:35: note: in expansion of macro 'LOGF'
  624 | #define RELEASE_LOG(channel, ...) LOGF(channel, 4, __VA_ARGS__)
      |                                   ^~~~
WTF/Headers/wtf/Assertions.h:643:69: note: in expansion of macro 'RELEASE_LOG'
  643 | #define RELEASE_LOG_IF(isAllowed, channel, ...) do { if (isAllowed) RELEASE_LOG(channel, __VA_ARGS__); } while (0)
      |                                                                     ^~~~~~~~~~~
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1693:40: note: in expansion of macro 'RELEASE_LOG_IF'
 1693 | #define LOCAL_LOG_IF_ALLOWED(fmt, ...) RELEASE_LOG_IF(networkStorageSession.sessionID().isAlwaysOnLoggingAllowed(), Network, "%p - %s::" fmt, loggedObject, label.characters(), ##__VA_ARGS__)
      |                                        ^~~~~~~~~~~~~~
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1695:5: note: in expansion of macro 'LOCAL_LOG_IF_ALLOWED'
 1695 |     LOCAL_LOG_IF_ALLOWED("logCookieInformation: webPageID=%s, frameID=%s, resourceID=%s: " str, escapedPageID.utf8().data(), escapedFrameID.utf8().data(), escapedIdentifier.utf8().data(), ##__VA_ARGS__)
      |     ^~~~~~~~~~~~~~~~~~~~
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1731:9: note: in expansion of macro 'LOCAL_LOG'
 1731 |         LOCAL_LOG(R"(  }%" PUBLIC_LOG_STRING ")", trailingComma);
      |         ^~~~~~~~~
In file included from DerivedSources/WebKit/unified-sources/UnifiedSource-72468c22-3.cpp:1:
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1731:26: note: format string is defined here
 1731 |         LOCAL_LOG(R"(  }%" PUBLIC_LOG_STRING ")", trailingComma);
      |                          ^
In file included from WTF/Headers/wtf/StdLibExtras.h:33,
                 from WTF/Headers/wtf/FastMalloc.h:26,
                 from Source/WebKit/config.h:42,
                 from Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:26,
                 from DerivedSources/WebKit/unified-sources/UnifiedSource-72468c22-3.cpp:1:
WTF/Headers/wtf/Assertions.h:621:25: warning: too many arguments for format [-Wformat-extra-args]
  621 |         fprintf(stderr, "[%s:%s:%i] " fmt "\n", logChannel.subsystem, logChannel.name, priority, ##__VA_ARGS__); \
      |                         ^~~~~~~~~~~~~~~~~~~~~~
WTF/Headers/wtf/Assertions.h:624:35: note: in expansion of macro 'LOGF'
  624 | #define RELEASE_LOG(channel, ...) LOGF(channel, 4, __VA_ARGS__)
      |                                   ^~~~
WTF/Headers/wtf/Assertions.h:643:69: note: in expansion of macro 'RELEASE_LOG'
  643 | #define RELEASE_LOG_IF(isAllowed, channel, ...) do { if (isAllowed) RELEASE_LOG(channel, __VA_ARGS__); } while (0)
      |                                                                     ^~~~~~~~~~~
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1693:40: note: in expansion of macro 'RELEASE_LOG_IF'
 1693 | #define LOCAL_LOG_IF_ALLOWED(fmt, ...) RELEASE_LOG_IF(networkStorageSession.sessionID().isAlwaysOnLoggingAllowed(), Network, "%p - %s::" fmt, loggedObject, label.characters(), ##__VA_ARGS__)
      |                                        ^~~~~~~~~~~~~~
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1695:5: note: in expansion of macro 'LOCAL_LOG_IF_ALLOWED'
 1695 |     LOCAL_LOG_IF_ALLOWED("logCookieInformation: webPageID=%s, frameID=%s, resourceID=%s: " str, escapedPageID.utf8().data(), escapedFrameID.utf8().data(), escapedIdentifier.utf8().data(), ##__VA_ARGS__)
      |     ^~~~~~~~~~~~~~~~~~~~
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1731:9: note: in expansion of macro 'LOCAL_LOG'
 1731 |         LOCAL_LOG(R"(  }%" PUBLIC_LOG_STRING ")", trailingComma);
      |         ^~~~~~~~~

I think that using {public]s and {private}s is something that's only supported by apple, and that's why the PUBLIC_LOG_STRING and PRIVATE_LOG_STRING are for, which are defined to just s for non apple ports, so your fix is in the right direction. But there must be something with the usage of those macros that's not fitting well somehow.

A fast solution would be just replacing {public}s and {private}s with just s, instead of using the macros.

I've also seen some {public]d and {private]d around, which can't be fixed with macros and it doesn't seem that there's any defined for integers. We should probably use just d there.

[Scony]

You're right - it looks like I've fixed the logs, but the warnings are still there which I missed. I'll explore that further.

[magomez]

There's PR https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1295 to fix this, so closing.