| Summary: | a lot gcc warnings because of %{public}s format specifier | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Víctor M. Jáquez L. <vjaquez> | ||||||
| Component: | Web Template Framework | Assignee: | Víctor M. Jáquez L. <vjaquez> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | beidson, benjamin, bfulgham, cdumez, cmarcelo, commit-queue, darin, dbates, eric.carlson, ews-watchlist, glenn, hta, japhet, jer.noble, krollin, mjs, philipj, sergio, thorton, tommyw, webkit-bug-importer, wilander | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=225137 | ||||||||
| Attachments: |
|
||||||||
Created attachment 390262 [details]
Patch
Comment on attachment 390262 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390262&action=review > Source/WTF/wtf/Assertions.h:487 > +#define LOGDSTR "s" In WebKit coding style we normally prefer words over clumps of letters. I don’t know that LOGDSTR is a good choice for the project. Is this referring to something named "logd"? Maybe PUBLIC_LOG_STRING would be a good name? Also, why not put the "%" into the macro? (In reply to Darin Adler from comment #2) > Also, why not put the "%" into the macro? I think that leaving the "%" out of the macro follows standard macros like PRId64. Any naming convention should take into account other types of format specifications that might be used. For instance: PUBLIC_LOG_STRING: {public}s PRIVATE_LOG_STRING: {private}s FUTUREOPTION_LOG_STRING: {future-option}s PUBLIC_LOG_OBJCOBJECT: {public}@ PRIVATE_LOG_OBJCOBJECT: {private}@ FUTUREOPTION_LOG_OBJCOBJECT: {future-option}@ So while I like LOGDSTR for its brevity, it doesn't scale. (In reply to Keith Rollin from comment #3) > (In reply to Darin Adler from comment #2) > > Also, why not put the "%" into the macro? > > I think that leaving the "%" out of the macro follows standard macros like > PRId64. It does. That alone doesn’t seem like reason to follow suit, though. (In reply to Darin Adler from comment #4) > (In reply to Keith Rollin from comment #3) > > (In reply to Darin Adler from comment #2) > > > Also, why not put the "%" into the macro? > > > > I think that leaving the "%" out of the macro follows standard macros like > > PRId64. > > It does. That alone doesn’t seem like reason to follow suit, though. What's the reason for not following suit? Personally, I'd like to be consistent with the standard. That way I don't have to remember "OK, add the % for the standard macros, but leave it out for WebKit macros. Or is it the other way around...". (In reply to Keith Rollin from comment #5) > (In reply to Darin Adler from comment #4) > > (In reply to Keith Rollin from comment #3) > > > (In reply to Darin Adler from comment #2) > > > > Also, why not put the "%" into the macro? > > > > > > I think that leaving the "%" out of the macro follows standard macros like > > > PRId64. > > > > It does. That alone doesn’t seem like reason to follow suit, though. > > What's the reason for not following suit? > > Personally, I'd like to be consistent with the standard. That way I don't > have to remember "OK, add the % for the standard macros, but leave it out > for WebKit macros. Or is it the other way around...". I do agree. It is better, IMO, to be consistent with other formatting macros. Created attachment 390505 [details]
Patch
(In reply to Keith Rollin from comment #3) > (In reply to Darin Adler from comment #2) > > Also, why not put the "%" into the macro? > > I think that leaving the "%" out of the macro follows standard macros like > PRId64. > > Any naming convention should take into account other types of format > specifications that might be used. For instance: > > PUBLIC_LOG_STRING: {public}s Done. Now this symbol is used. > PRIVATE_LOG_STRING: {private}s > FUTUREOPTION_LOG_STRING: {future-option}s > > PUBLIC_LOG_OBJCOBJECT: {public}@ > PRIVATE_LOG_OBJCOBJECT: {private}@ > FUTUREOPTION_LOG_OBJCOBJECT: {future-option}@ > > So while I like LOGDSTR for its brevity, it doesn't scale. (In reply to Keith Rollin from comment #5) > What's the reason for not following suit? To me PUBLIC_LOG_STRING doesn’t seem much like PRId64. I agree that if you thought of them as similar and closely related then we’d want them to match. > Personally, I'd like to be consistent with the standard. That way I don't > have to remember "OK, add the % for the standard macros, but leave it out > for WebKit macros. Or is it the other way around...". OK, tastes differ, and I certainly won’t insist. May I have r+ & q+ ? O:) Comment on attachment 390505 [details] Patch Clearing flags on attachment: 390505 Committed r256745: <https://trac.webkit.org/changeset/256745> All reviewed patches have been landed. Closing bug. |
While compiling with GCC a lot of big warnings are shown because those macros boil down to vfprintf() in Linux, and that {public} specifier is invalid. It is only for os_log [1] 1. https://developer.apple.com/documentation/os/logging?language=objc