Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Review: Needs Information
Should the implementation of base64:decode#1 be:
declare function base64:decode($base64 as xs:base64Binary) as xs:string {
base64:decode($base64, UTF-8)};
?
--
https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
I don't believe it can be done that way, because the base64 module is an internally-implemented module. In release mode, the .xq file for the module won't even be parsed, so it cannot contain user-defined functions. Matthias, correct me if I'm wrong. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Approved = Needs review For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
yes correct, sorry I missed that. On Tue, May 8, 2012 at 12:23 PM, Chris Hillery chillery+launch...@lambda.nu wrote: I don't believe it can be done that way, because the base64 module is an internally-implemented module. In release mode, the .xq file for the module won't even be parsed, so it cannot contain user-defined functions. Matthias, correct me if I'm wrong. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 You are reviewing the proposed merge of lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug-996084-reuse-stream-2012-05-08T10-33-54.117Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Validation queue job bug-996084-reuse-stream-2012-05-08T10-33-54.117Z is finished. The final status was: All tests succeeded! -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1, Needs Fixing 1, Pending 1. Got: 2 Approve, 2 Pending. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Approved = Needs review For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
I don't believe it can be done that way, because the base64 module is an internally-implemented module. In release mode, the .xq file for the module won't even be parsed, so it cannot contain user-defined functions. Matthias, correct me if I'm wrong. It could but it would require us to process the .xq file at runtime which I wanted to avoid given the fact that it doesn't make the C++ implementation too complex. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Review: Approve I tested it and it works. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug-996084-reuse-stream-2012-05-09T00-51-58.501Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Validation queue job bug-996084-reuse-stream-2012-05-09T00-51-58.501Z is finished. The final status was: All tests succeeded! -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Chris Hillery has proposed merging
lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba.
Requested reviews:
Chris Hillery (ceejatec)
Matthias Brantner (matthias-brantner)
Related bugs:
Bug #996084 in Zorba: crash in Streamable*Item with file module
https://bugs.launchpad.net/zorba/+bug/996084
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105016
--
https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105016
Your team Zorba Coders is subscribed to branch lp:zorba.
=== modified file 'src/runtime/base64/base64_impl.cpp'
--- src/runtime/base64/base64_impl.cpp 2012-05-03 12:31:51 +
+++ src/runtime/base64/base64_impl.cpp 2012-05-08 01:12:21 +
@@ -52,11 +52,9 @@
else
{
// streamable string eventually transcoding
- GENV_ITEMFACTORY-createStreamableString(
+ GENV_ITEMFACTORY-createSharedStreamableString(
result,
- lItem-getStream(),
- lItem-getStreamReleaser(),
- lItem-isSeekable());
+ lItem);
}
}
else
=== modified file 'src/store/api/item_factory.h'
--- src/store/api/item_factory.h 2012-05-03 12:31:51 +
+++ src/store/api/item_factory.h 2012-05-08 01:12:21 +
@@ -97,6 +97,19 @@
bool seekable = false) = 0;
/**
+ * Create a StreamableStringItem which re-uses the stream from another
+ * Streamable*Item. This will maintain a reference to the original
+ * item to ensure the stream is not cleaned up before we are done with it.
+ *
+ * It only makes sense to use this method if either (a) the dependent item's
+ * stream is seekable and hence re-usable, or (b) you are sure that the
+ * dependent item will not be utilized after this new item is created.
+ */
+ virtual bool createSharedStreamableString(
+ Item_t result,
+ Item_t streamble_dependent) = 0;
+
+ /**
* Specification: [http://www.w3.org/TR/xmlschema-2/#normalizedString]
* @param value string representation of the value
*/
=== modified file 'src/store/naive/atomic_items.cpp'
--- src/store/naive/atomic_items.cpp 2012-05-04 14:32:28 +
+++ src/store/naive/atomic_items.cpp 2012-05-08 01:12:21 +
@@ -1673,6 +1673,36 @@
/***
class StreamableStringItem
/
+StreamableStringItem::StreamableStringItem(
+std::istream aStream,
+StreamReleaser streamReleaser,
+bool seekable) :
+ theIstream(aStream),
+ theIsMaterialized(false),
+ theIsConsumed(false),
+ theIsSeekable(seekable),
+ theStreamReleaser(streamReleaser),
+ theStreamableDependent(nullptr)
+{
+}
+
+StreamableStringItem::StreamableStringItem(
+store::Item_t aStreamableDependent) :
+ theIstream(aStreamableDependent-getStream()),
+ theIsMaterialized(false),
+ theIsConsumed(false),
+ theIsSeekable(aStreamableDependent-isSeekable()),
+ theStreamReleaser(nullptr),
+ theStreamableDependent(aStreamableDependent)
+{
+ ZORBA_ASSERT(theStreamableDependent-isStreamable());
+
+ // We copied the dependent item's stream and seekable flag in the initializer
+ // above, but did NOT copy the StreamReleaser. The dependent item maintains
+ // memory ownership of the stream in this way.
+}
+
+
void StreamableStringItem::appendStringValue(zstring aBuf) const
{
if (!theIsMaterialized)
=== modified file 'src/store/naive/atomic_items.h'
--- src/store/naive/atomic_items.h 2012-05-03 12:31:51 +
+++ src/store/naive/atomic_items.h 2012-05-08 01:12:21 +
@@ -875,6 +875,8 @@
StreamReleaser theStreamReleaser;
+ store::Item_t theStreamableDependent;
+
public:
bool equals(
store::Item const*,
@@ -922,15 +924,10 @@
StreamableStringItem(
std::istream aStream,
StreamReleaser streamReleaser,
- bool seekable = false)
-:
-theIstream(aStream),
-theIsMaterialized(false),
-theIsConsumed(false),
-theIsSeekable(seekable),
-theStreamReleaser(streamReleaser)
- {
- }
+ bool seekable = false);
+
+ StreamableStringItem(
+ store::Item_t aStreamableDependent);
void materialize() const;
};
=== modified file 'src/store/naive/simple_item_factory.cpp'
--- src/store/naive/simple_item_factory.cpp 2012-05-03 12:31:51 +
+++ src/store/naive/simple_item_factory.cpp 2012-05-08 01:12:21 +
@@ -157,6 +157,14 @@
return true;
}
+bool BasicItemFactory::createSharedStreamableString(
+store::Item_t result,
+store::Item_t streamable_dependent)
+{
+ result = new StreamableStringItem( streamable_dependent );
+ return true;
+}
+
bool BasicItemFactory::createNormalizedString(store::Item_t result, zstring value)
{
=== modified file 'src/store/naive/simple_item_factory.h'
--- src/store/naive/simple_item_factory.h 2012-05-03 12:31:51 +
+++ src/store/naive/simple_item_factory.h 2012-05-08 01:12:21 +
@@ -106,6 +106,10 @@
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105016 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Two things I'm not totally sure/happy about: 1. The name createSharedStreamableString(). Probably createSharingStreamableString() would be more apt, but it doesn't seem clear to me. Maybe I should have just stuck with overloading createStreamableString(). Thoughts? 2. I had to initialize theIstream in the initializer block of the new StreamableStringItem constructor, because theIstream is a reference. Therefore, the ZORBA_ASSERT() in that constructor will probably never go off - if the streamable_dependent isn't streamable, we'll have already thrown an exception in the call to getStream() in the initializer block. I can't see a way around this. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105016 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Commit Message changed to: - Add the ability to create a StreamableStringItem that re-uses the stream from another Streamable*Item (in a memory-ownership-safe way). Fixed bug #996084 (crash in Streamable*Item with file module) - Add a base64:decode#2 function that also does transcoding to utf-8 For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug-996084-reuse-stream-2012-05-08T03-38-00.431Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1, Needs Fixing 1, Pending 1. Got: 1 Approve, 3 Pending. -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/bug-996084-reuse-stream into lp:zorba has been updated. Status: Approved = Needs review For more details, see: https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 -- https://code.launchpad.net/~zorba-coders/zorba/bug-996084-reuse-stream/+merge/105025 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
