Hi Brad, > We need to find a way to get whatever changes you think are ready to go into > openchange. Some of it looks quite general, but some if it is beyond me.
I have published the whole implementation progress in a public repository - http://repo.or.cz/w/OpenChange-git-clone.git?a=shortlog;h=refs/heads/nspi_impl - so that you can pick patches you like. It is not an SVN repository, sorry for this, but I think it should be easy to publish those patches in SVN branch somewhere (I have cloned Open Change SVN repo using git-svn). > > Would it be possible to have you put patches as individual tickets onto > trac.openchange.org? At least some of the patch doesn't look applicable > (e.g. > the changes to script/installsamba4.sh), but I do understand some of it > and > would be happy to apply those parts (e.g. the changes to fix warning > codes > and the macro definitions). Yes of course. I've registered an account in ticketing system but I can't create new ticket? How should I do this? > > Also, I think MAPI_STATUS_IS_ERR() macro should be reimplemented. It is > > currently based on NTSTATUS code mapping which is not correct as NTSTATUS > > uses the two most significant bits to indicate status. MAPISTATUS on the > > other hand uses only the most significant bit - either ERROR or SUCCESS > > (like HRESULT type). > I'm not so sure. I think the name is perhaps a bit confusing. Lets > look at this > in a bit more detail. I assume you are suggesting making this change: > --- libmapi/conf/mparse.pl (revision 1466) > +++ libmapi/conf/mparse.pl (working copy) > @@ -555,7 +555,7 @@ > mparse "#define MAPI_STATUS_V(x) ((SCODE)x)"; > mparse ""; > mparse "#define MAPI_STATUS_IS_OK(x) (MAPI_STATUS_V(x) == 0)"; > - mparse "#define MAPI_STATUS_IS_ERR(x) ((MAPI_STATUS_V(x) & > 0xc0000000) == 0xc0000000)"; > + mparse "#define MAPI_STATUS_IS_ERR(x) ((MAPI_STATUS_V(x) & > 0x80000000) == 0x80000000)"; > mparse "#define MAPI_STATUS_EQUAL(x,y) (MAPI_STATUS_V(x) == > MAPI_STATUS_V(y))"; > mparse ""; > mparse "#define MAPI_STATUS_IS_OK_RETURN(x) do { \\"; > > I can't see anywhere MAPI_STATUS_IS_ERR() is used, but working off the > similar MAPI_STATUS_IS_OK(), we can see it is only used on > NT_STATUS_V() > (i.e. the value of the NTSTATUS). So perhaps this is only intended to > work > on NTSTATUS, not on MAPISTATUS? > > I'm not really sure, but perhaps this is intentional? > In my opinion, MAPI_STATUS_IS_ERR() is not been used because it is not working. >From my windows background, I prefer to use asserts like >"assert(!MAPI_STATUS_IS_ERR(ret));" to note explicitly in the code that I am >asserting on errors. In windows headers MAPISTATUS (and HRESULT) are defined >as follows: = 0 -> definitely a SUCCESS < 0 -> definitely an ERROR > 0 -> SUCCESS with WARNINGS This should lead to following patch for "mparse.pl": Index: libmapi/conf/mparse.pl =================================================================== --- libmapi/conf/mparse.pl (revision 1448) +++ libmapi/conf/mparse.pl (working copy) @@ -539,23 +539,23 @@ mparse "} while (0);"; mparse ""; mparse "/* Status macros for MAPI */"; - mparse "typedef unsigned long SCODE;"; + mparse "typedef long SCODE;"; mparse ""; mparse ""; mparse "#define SEVERITY_ERROR 1"; - mparse "#define SEVERITY_WARN 0"; + mparse "#define SEVERITY_SUCCESS 0"; mparse ""; mparse "#define FACILITY_ITF 4"; mparse "#define MAKE_MAPI_CODE(sev, fac, code) \\"; - mparse "(((SCODE)(sev)<<31)|((SCODE)(fac)<<16)|((SCODE)(code)))"; + mparse "((SCODE) (((unsigned long)(sev)<<31)|((unsigned long)(fac)<<16)|((unsigned long)(code))) )"; mparse ""; mparse "#define MAKE_MAPI_E(code) (MAKE_MAPI_CODE(SEVERITY_ERROR, FACILITY_ITF, code))"; - mparse "#define MAKE_MAPI_S(code) (MAKE_MAPI_CODE(SEVERITY_WARN, FACILITY_ITF, code))"; + mparse "#define MAKE_MAPI_S(code) (MAKE_MAPI_CODE(SEVERITY_SUCCESS, FACILITY_ITF, code))"; mparse ""; mparse "#define MAPI_STATUS_V(x) ((SCODE)x)"; mparse ""; - mparse "#define MAPI_STATUS_IS_OK(x) (MAPI_STATUS_V(x) == 0)"; - mparse "#define MAPI_STATUS_IS_ERR(x) ((MAPI_STATUS_V(x) & 0xc0000000) == 0xc0000000)"; + mparse "#define MAPI_STATUS_IS_OK(x) (MAPI_STATUS_V(x) >= 0)"; + mparse "#define MAPI_STATUS_IS_ERR(x) ((MAPI_STATUS_V(x) < 0)"; mparse "#define MAPI_STATUS_EQUAL(x,y) (MAPI_STATUS_V(x) == MAPI_STATUS_V(y))"; mparse ""; mparse "#define MAPI_STATUS_IS_OK_RETURN(x) do { \\"; I've been concerned a little bit about side-effects after changing the SCODE definition to a signed long, but as far as I can see, SCODE type is not used explicitly anywhere in the code. So this patch should be harmless for the existing code base. > > In resume: currently Outlook works quite well with this set of Nspi calls > > impelemente. One can use "check names" feature and Address Book feature. > While I don't claim to understand the code, I'd like to offer most > sincere > congratulations on getting this working. Very nice indeed. Thanks a lot :) BR, Kamen Mazdrashki [email protected] CISCO SYSTEMS BULGARIA EOOD 18 Macedonia Blvd. Sofia 1606 Bulgaria http://www.cisco.com/global/BG/ _______________________________________________ devel mailing list [email protected] http://mailman.openchange.org/listinfo/devel
