Quantcast

upstreaming SpiderShim customizations to DeflateStringToUTF8Buffer

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

upstreaming SpiderShim customizations to DeflateStringToUTF8Buffer

Myk Melez-4

SpiderShim currently has a custom implementation of
DeflateStringToUTF8Buffer [1], since neither of the existing ones (the
public one in CharacterEncoding and a private one in CTypes) does quite
what it wants, which is to return a partial result if the destination
buffer runs out of space (CharacterEncoding requires the buffer to have
enough space [2]) and continue on a bad surrogate (CTypes aborts on a
bad surrogate [3]).

It seems like it should be possible to refactor all three
implementations into a single, public one, or at least to upstream the
SpiderShim customizations into the public implementation in
CharacterEncoding (leaving the private implementation in CTypes alone).
Would the SpiderMonkey team be amenable to that? And is this the right
forum for such a suggestion, or would it be better to file a bug and
discuss the proposal there?

-myk

[1]
https://github.com/mozilla/spidernode/blob/master/deps/spidershim/src/v8string.cc#L455-L538
[2]
https://dxr.mozilla.org/mozilla-central/rev/e5a10b/js/public/CharacterEncoding.h#208-213
[3]
https://dxr.mozilla.org/mozilla-central/rev/e5a10b/js/src/ctypes/CTypes.cpp#172-177

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: upstreaming SpiderShim customizations to DeflateStringToUTF8Buffer

Terrence Cole-3
Yes, absolutely (to both questions)! The implementation in
CharacterEncoding is provided as a convenience to embedders (given that
most text format converter libraries don't support JS's weirdo text
format); if it is inconvenient for embedding then we should definitely
enhance it. If you already have working code, even better.

Cheers,
Terrence

On Fri, May 6, 2016 at 11:20 AM, Myk Melez <[hidden email]> wrote:

>
> SpiderShim currently has a custom implementation of
> DeflateStringToUTF8Buffer [1], since neither of the existing ones (the
> public one in CharacterEncoding and a private one in CTypes) does quite
> what it wants, which is to return a partial result if the destination
> buffer runs out of space (CharacterEncoding requires the buffer to have
> enough space [2]) and continue on a bad surrogate (CTypes aborts on a bad
> surrogate [3]).
>
> It seems like it should be possible to refactor all three implementations
> into a single, public one, or at least to upstream the SpiderShim
> customizations into the public implementation in CharacterEncoding (leaving
> the private implementation in CTypes alone). Would the SpiderMonkey team be
> amenable to that? And is this the right forum for such a suggestion, or
> would it be better to file a bug and discuss the proposal there?
>
> -myk
>
> [1]
> https://github.com/mozilla/spidernode/blob/master/deps/spidershim/src/v8string.cc#L455-L538
> [2]
> https://dxr.mozilla.org/mozilla-central/rev/e5a10b/js/public/CharacterEncoding.h#208-213
> [3]
> https://dxr.mozilla.org/mozilla-central/rev/e5a10b/js/src/ctypes/CTypes.cpp#172-177
>
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: upstreaming SpiderShim customizations to DeflateStringToUTF8Buffer

Myk Melez-4
> Terrence Cole <mailto:[hidden email]>
> 2016 May 6 at 17:07
> Yes, absolutely (to both questions)!
Woot! In that case, I have a few more such issues, but I'll start
separate threads on them.

> The implementation in
> CharacterEncoding is provided as a convenience to embedders (given that
> most text format converter libraries don't support JS's weirdo text
> format); if it is inconvenient for embedding then we should definitely
> enhance it. If you already have working code, even better.
I do have working code, although it isn't a drop-in replacement for the
existing function, so it'll need refactoring. I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1271014 on this.

-myk

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Loading...