G'day team,
After some investigation into an error reported by valgrind, I'm proposing an
API change for the MoveCopyMessages() function.
The signature is currently
enum MAPISTATUS MoveCopyMessages(mapi_object_t *obj_src, mapi_object_t
*obj_dst, mapi_id_t *message_id, bool WantCopy)
The problem is the message_id array. The subtle part is that it needs to be
terminated with a null mapi_id_t*, because of this:
for (request.count = 0; message_id[request.count]; request.count++);
That is a bit error prone - enough so to be wrong in the mapitest.
Now we could just fix the mapitest and document the requirements, or we could
change the signature.
Two ideas for a different signature:
1. We could add a uint16_t count to the signature, that says how long the
array is.
2. We could use a better data structure than mapi_id_t*. I'm suggesting
mapi_id_array_t:
enum MAPISTATUS MoveCopyMessages(mapi_object_t *obj_src,
mapi_object_t *obj_dst,
mapi_id_array_t *message_id,
bool WantCopy)
Attached is the total patch, including changes to mapitest and torture tests.
There is a bit of extra code in the mapitest (Step 6 - Step 8) that
cleans up the moved messages, which is required in any case.
I'm going to commit this on Tuesday night (my time - UTC+10). Let
me know if you have any concerns or comments.
Brad
Index: libmapi/IMAPIFolder.c
===================================================================
--- libmapi/IMAPIFolder.c (revision 597)
+++ libmapi/IMAPIFolder.c (working copy)
@@ -356,7 +356,7 @@
\param obj_src The source folder
\param obj_dst The destination folder
- \param message_id pointer on an array of message IDs
+ \param message_id pointer to container object for message ids.
\param WantCopy boolean value, defines whether the operation is a
copy or a move
@@ -376,7 +376,7 @@
*/
_PUBLIC_ enum MAPISTATUS MoveCopyMessages(mapi_object_t *obj_src,
mapi_object_t *obj_dst,
- mapi_id_t *message_id,
+ mapi_id_array_t *message_id,
bool WantCopy)
{
NTSTATUS status;
@@ -394,18 +394,15 @@
size = 0;
- /* FIXME: the function can take an array of msgids and is not
- * limited to a unique msgid
- */
-
/* Fill the CopyMessage operation */
request.handle_idx = 0x1;
size += sizeof (uint8_t);
- for (request.count = 0; message_id[request.count]; request.count++);
+ request.count = message_id->count;
size += sizeof (uint16_t);
- request.message_id = message_id;
+ retval = mapi_id_array_get(mem_ctx, message_id, &(request.message_id));
+ MAPI_RETVAL_IF(retval, retval, mem_ctx);
size += request.count * sizeof (mapi_id_t);
request.WantAsynchronous = 0;
Index: utils/mapitest/modules/module_oxcfold.c
===================================================================
--- utils/mapitest/modules/module_oxcfold.c (revision 594)
+++ utils/mapitest/modules/module_oxcfold.c (working copy)
@@ -376,7 +376,13 @@
mapi_object_t obj_folder_src;
mapi_object_t obj_folder_dst;
mapi_object_t obj_message;
- mapi_id_t msgid[3];
+ mapi_object_t dst_contents;
+ uint32_t dst_count;
+ struct mapi_SRestriction res;
+ struct SPropTagArray *SPropTagArray;
+ struct SRowSet SRowSet;
+ mapi_id_array_t msg_id_array;
+ mapi_id_t msgid[20];
mapi_id_t id_folder;
uint32_t i;
@@ -403,8 +409,15 @@
if (GetLastError() != MAPI_E_SUCCESS) {
return false;
}
+ mapi_object_init(&(dst_contents));
+ GetContentsTable(&(obj_folder_dst), &(dst_contents), 0, &dst_count);
+ if (GetLastError() != MAPI_E_SUCCESS) {
+ mapitest_print(mt, "* %-35s: 0x%.8x\n", "GetContentsTable",
GetLastError());
+ ret = false;
+ }
/* Step 4. Create sample messages */
+ mapi_id_array_init(&msg_id_array);
for (i = 0; i < 3; i++) {
mapi_object_init(&obj_message);
retval = mapitest_common_message_create(mt, &obj_folder_src,
&obj_message, MT_MAIL_SUBJECT);
@@ -416,17 +429,49 @@
if (retval != MAPI_E_SUCCESS) {
ret = false;
}
- msgid[i] = mapi_object_get_id(&obj_message);
+ mapi_id_array_add_obj(&msg_id_array, &obj_message);
mapi_object_release(&obj_message);
}
/* Step 5. Move messages from source to destination */
- retval = MoveCopyMessages(&obj_folder_src, &obj_folder_dst, msgid, 0);
+ retval = MoveCopyMessages(&obj_folder_src, &obj_folder_dst,
&msg_id_array, 0);
mapitest_print(mt, "* %-35s: 0x%.8x\n", "MoveCopyMessages",
GetLastError());
if (retval != MAPI_E_SUCCESS) {
ret = false;
}
+ mapi_id_array_release(&msg_id_array);
+ /* Step 6. Apply a filter */
+ res.rt = RES_PROPERTY;
+ res.res.resProperty.relop = RES_PROPERTY;
+ res.res.resProperty.ulPropTag = PR_SUBJECT;
+ res.res.resProperty.lpProp.ulPropTag = PR_SUBJECT;
+ res.res.resProperty.lpProp.value.lpszA = MT_MAIL_SUBJECT;
+
+ Restrict(&(dst_contents), &res);
+ mapitest_print(mt, "* %-35s: 0x%.8x\n", "Restrict", GetLastError());
+ if (GetLastError() != MAPI_E_SUCCESS) {
+ ret = false;
+ }
+
+ /* Step 7. Get the filtered row */
+ SPropTagArray = set_SPropTagArray(mt->mem_ctx, 0x1, PR_MID);
+ SetColumns(&(dst_contents), SPropTagArray);
+ mapitest_print(mt, "* %-35s: 0x%.8x\n", "SetColumns", GetLastError());
+ MAPIFreeBuffer(SPropTagArray);
+
+ QueryRows(&(dst_contents), 20, TBL_NOADVANCE, &SRowSet);
+ mapitest_print(mt, "* %-35s: 0x%.8x\n", "QueryRows", GetLastError());
+ if ( (retval == MAPI_E_SUCCESS) && (SRowSet.cRows > 0) ) {
+ for (i = 0; i < SRowSet.cRows; ++i) {
+ msgid[i] = SRowSet.aRow[i].lpProps[0].value.d;
+ }
+ }
+
+ /* Step 8. Delete Messages */
+ retval = DeleteMessage(&obj_folder_dst, msgid, i);
+ mapitest_print(mt, "* %-35s: 0x%.8x\n", "DeleteMessage",
GetLastError());
+
/* Release */
mapi_object_release(&obj_folder_src);
mapi_object_release(&obj_folder_dst);
Index: torture/mapi_copymail.c
===================================================================
--- torture/mapi_copymail.c (revision 594)
+++ torture/mapi_copymail.c (working copy)
@@ -41,7 +41,7 @@
mapi_object_t obj_table;
mapi_id_t id_src;
mapi_id_t id_dst;
- mapi_id_t *msgid;
+ mapi_id_array_t msg_id_array;
struct SPropTagArray *SPropTagArray = NULL;
struct SRowSet rowset;
int i;
@@ -102,18 +102,20 @@
mapi_errstr("SetColumns", GetLastError());
if (retval != MAPI_E_SUCCESS) return false;
+ mapi_id_array_init(&msg_id_array);
+
while ((retval = QueryRows(&obj_table, 0xa, TBL_ADVANCE, &rowset)) !=
MAPI_E_NOT_FOUND && rowset.cRows) {
- msgid = talloc_array(mem_ctx, uint64_t, rowset.cRows);
for (i = 0; i < rowset.cRows; i++) {
- msgid[i] = rowset.aRow[i].lpProps[1].value.d;
+ mapi_id_array_add_id(&msg_id_array,
rowset.aRow[i].lpProps[1].value.d);
}
- retval = MoveCopyMessages(&obj_dir_src, &obj_dir_dst, msgid, 1);
+ retval = MoveCopyMessages(&obj_dir_src, &obj_dir_dst,
&msg_id_array, 1);
mapi_errstr("MoveCopyMessages", GetLastError());
if (retval != MAPI_E_SUCCESS) return false;
}
/* release mapi objects
*/
+ mapi_id_array_release(&msg_id_array);
mapi_object_release(&obj_table);
mapi_object_release(&obj_dir_src);
mapi_object_release(&obj_dir_dst);
_______________________________________________
devel mailing list
[email protected]
http://mailman.openchange.org/listinfo/devel
