Bug 165070 – Fix build warnings in WebCore/Modules/indexeddb/server/IDBSerialization.cp

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

[Csaba Osztrogonác]

../../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.

[Csaba Osztrogonác]

Created attachment 295421 [details]
Patch

[Csaba Osztrogonác]

Created attachment 295422 [details]
Patch

[Csaba Osztrogonác]

Created attachment 295424 [details]
Patch

[Brady Eidson]

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!

[WebKit Commit Bot]

Comment on attachment 295424 [details]
Patch

Clearing flags on attachment: 295424

Committed r208984: <http://trac.webkit.org/changeset/208984>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(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?

[Carlos Garcia Campos]

(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.

[Brady Eidson]

(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!

[Carlos Garcia Campos]

(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?

[Brady Eidson]

(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.

[Carlos Garcia Campos]

(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.