Bug 165070 - Fix build warnings in WebCore/Modules/indexeddb/server/IDBSerialization.cp
Summary: Fix build warnings in WebCore/Modules/indexeddb/server/IDBSerialization.cp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 02:52 PST by Csaba Osztrogonác
Modified: 2016-11-30 02:59 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.72 KB, patch)
2016-11-25 02:53 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (2.66 KB, patch)
2016-11-25 07:38 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (2.92 KB, patch)
2016-11-25 08:01 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2016-11-25 02:52:13 PST
../../Source/WebCore/Modules/indexeddb/server/IDBSerialization.cpp:99:22: warning: unused variable 'LegacySerializedKeyVersion' [-Wunused-const-variable]
static const uint8_t LegacySerializedKeyVersion = 'b';
                     ^
../../Source/WebCore/Modules/indexeddb/server/IDBSerialization.cpp:137:22: warning: unused variable 'SIDBKeyVersion' [-Wunused-const-variable]
static const uint8_t SIDBKeyVersion = 0x00;
                     ^
../../Source/WebCore/Modules/indexeddb/server/IDBSerialization.cpp:219:13: warning: function 'encodeKey' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static void encodeKey(Vector<char>& data, const IDBKeyData& key)
            ^
../../Source/WebCore/Modules/indexeddb/server/IDBSerialization.cpp:284:13: warning: function 'decodeKey' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static bool decodeKey(const uint8_t*& data, const uint8_t* end, IDBKeyData& result)
            ^
4 warnings generated.


All of these are used only if USE(CF) is true, let's guard them.
Comment 1 Csaba Osztrogonác 2016-11-25 02:53:13 PST
Created attachment 295421 [details]
Patch
Comment 2 Csaba Osztrogonác 2016-11-25 07:38:31 PST
Created attachment 295422 [details]
Patch
Comment 3 Csaba Osztrogonác 2016-11-25 08:01:36 PST
Created attachment 295424 [details]
Patch
Comment 4 Brady Eidson 2016-11-26 20:16:17 PST
Comment on attachment 295424 [details]
Patch

Any chance I could convince a Linux/GLib type person to dig up the magic first character for a GLib-encoded key? Then we could have Linux actually use the new serialization and only have to have one of these guards!
Comment 5 WebKit Commit Bot 2016-11-26 20:41:14 PST
Comment on attachment 295424 [details]
Patch

Clearing flags on attachment: 295424

Committed r208984: <http://trac.webkit.org/changeset/208984>
Comment 6 WebKit Commit Bot 2016-11-26 20:41:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Csaba Osztrogonác 2016-11-26 23:23:52 PST
(In reply to comment #4)
> Comment on attachment 295424 [details]
> Patch
> 
> Any chance I could convince a Linux/GLib type person to dig up the magic
> first character for a GLib-encoded key? Then we could have Linux actually
> use the new serialization and only have to have one of these guards!

Maybe Carlos?
Comment 8 Carlos Garcia Campos 2016-11-27 01:38:22 PST
(In reply to comment #7)
> (In reply to comment #4)
> > Comment on attachment 295424 [details]
> > Patch
> > 
> > Any chance I could convince a Linux/GLib type person to dig up the magic
> > first character for a GLib-encoded key? Then we could have Linux actually
> > use the new serialization and only have to have one of these guards!
> 
> Maybe Carlos?

Sure, I didn't know anything about this, I'll look at it.
Comment 9 Brady Eidson 2016-11-27 16:43:01 PST
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #4)
> > > Comment on attachment 295424 [details]
> > > Patch
> > > 
> > > Any chance I could convince a Linux/GLib type person to dig up the magic
> > > first character for a GLib-encoded key? Then we could have Linux actually
> > > use the new serialization and only have to have one of these guards!
> > 
> > Maybe Carlos?
> 
> Sure, I didn't know anything about this, I'll look at it.

The CFPropertyList serialization literally always starts with the first character 'b'
I'm hoping the Glib serialization literally always starts with some other magic character. (Or, even if it happened to be 'b', that'd be fine)

Once we confirm it does, then Glib can adopt the new serialization right now, no more questions asked!
Comment 10 Carlos Garcia Campos 2016-11-28 00:40:26 PST
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #4)
> > > > Comment on attachment 295424 [details]
> > > > Patch
> > > > 
> > > > Any chance I could convince a Linux/GLib type person to dig up the magic
> > > > first character for a GLib-encoded key? Then we could have Linux actually
> > > > use the new serialization and only have to have one of these guards!
> > > 
> > > Maybe Carlos?
> > 
> > Sure, I didn't know anything about this, I'll look at it.
> 
> The CFPropertyList serialization literally always starts with the first
> character 'b'
> I'm hoping the Glib serialization literally always starts with some other
> magic character. (Or, even if it happened to be 'b', that'd be fine)
> 
> Once we confirm it does, then Glib can adopt the new serialization right
> now, no more questions asked!

No, we use GVariant that doesn't add any magic character, but I guess the point is detecting if given data was encoded using the KeyedEncoder right? In that case we could at least check if the data is a GVariant dictionary and assume that it was encoded by KeyedEncoder in such case. The new serialization doesn't need any platform specific code, right?
Comment 11 Brady Eidson 2016-11-29 18:41:55 PST
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #7)
> > > > (In reply to comment #4)
> > > > > Comment on attachment 295424 [details]
> > > > > Patch
> > > > > 
> > > > > Any chance I could convince a Linux/GLib type person to dig up the magic
> > > > > first character for a GLib-encoded key? Then we could have Linux actually
> > > > > use the new serialization and only have to have one of these guards!
> > > > 
> > > > Maybe Carlos?
> > > 
> > > Sure, I didn't know anything about this, I'll look at it.
> > 
> > The CFPropertyList serialization literally always starts with the first
> > character 'b'
> > I'm hoping the Glib serialization literally always starts with some other
> > magic character. (Or, even if it happened to be 'b', that'd be fine)
> > 
> > Once we confirm it does, then Glib can adopt the new serialization right
> > now, no more questions asked!
> 
> No, we use GVariant that doesn't add any magic character, but I guess the
> point is detecting if given data was encoded using the KeyedEncoder right?

Right, which in the GLib case is GVariant, and I guess that doesn't have easy detection...?

> In that case we could at least check if the data is a GVariant dictionary
> and assume that it was encoded by KeyedEncoder in such case. 

That's the hard part - How to detect if it's a GVariant dictionary *efficiently*

> The new serialization doesn't need any platform specific code, right?

That's right, it's cross platform.
Comment 12 Carlos Garcia Campos 2016-11-30 02:59:57 PST
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > (In reply to comment #7)
> > > > > (In reply to comment #4)
> > > > > > Comment on attachment 295424 [details]
> > > > > > Patch
> > > > > > 
> > > > > > Any chance I could convince a Linux/GLib type person to dig up the magic
> > > > > > first character for a GLib-encoded key? Then we could have Linux actually
> > > > > > use the new serialization and only have to have one of these guards!
> > > > > 
> > > > > Maybe Carlos?
> > > > 
> > > > Sure, I didn't know anything about this, I'll look at it.
> > > 
> > > The CFPropertyList serialization literally always starts with the first
> > > character 'b'
> > > I'm hoping the Glib serialization literally always starts with some other
> > > magic character. (Or, even if it happened to be 'b', that'd be fine)
> > > 
> > > Once we confirm it does, then Glib can adopt the new serialization right
> > > now, no more questions asked!
> > 
> > No, we use GVariant that doesn't add any magic character, but I guess the
> > point is detecting if given data was encoded using the KeyedEncoder right?
> 
> Right, which in the GLib case is GVariant, and I guess that doesn't have
> easy detection...?
> 
> > In that case we could at least check if the data is a GVariant dictionary
> > and assume that it was encoded by KeyedEncoder in such case. 
> 
> That's the hard part - How to detect if it's a GVariant dictionary
> *efficiently*
> 
> > The new serialization doesn't need any platform specific code, right?
> 
> That's right, it's cross platform.

Ok, understood, thanks! I've attached a patch to bug #165191, it's not as efficient as checking data[0], but hopefully good enough.