+1 for trunk. Thanks. On May 31, 2012 11:28 PM, "Vladimir Berezniker" <v...@hitechman.com> wrote:
> Attached please find a followup patch that fixes issues you have raised > for > CPPADDR_NULL_PTR for other macros in JNIUtil.h, so that they become > consistent. > > [[[ > JavaHL: Make handling of expr and whitespace after ret_val parameters > consistent accross macros > > [ in subversion/bindings/javahl/native ] > > * JNIUtil.h > (SVN_JNI_NULL_PTR_EX): parenthesize expr for safety > (SVN_JNI_NULL_PTR_EX, SVN_JNI_ERR, POP_AND_RETURN): eliminate unnecessary > whitespace after ret_val > ]]] > > Regards, > > Vladimir > > On Thu, May 31, 2012 at 3:35 AM, Greg Stein <gst...@gmail.com> wrote: > >> On Thu, May 31, 2012 at 12:43 AM, Vladimir Berezniker >> <v...@hitechman.com> wrote: >> > Patch 01 - Introduce macro >> > >> > [[[ >> > JavaHL: Added CPPADDR_NULL_PTR macro to reduce amount of duplicate code >> > checking C++ pointer extracted from the java object >> > >> > [ in subversion/bindings/javahl/native ] >> > >> > * JNIUtil.h >> > (CPPADDR_NULL_PTR): New macro to test for NULL pointer and raise java >> > exception if necessary >> > ]]] >> >> Replying to just this patch. The second patch seems pretty mechanical. >> And we're only looking at minor nits. >> >> (sorry, but the patch doesn't inline into this response, so let's just >> be flexible here...) >> >> >> The macro argument substitutions need to be parenthesized for safety. >> So it would be: (expr) == NULL, and it would be: return (ret_val); >> >> Next bit: the indentation in the diff seems to be off. Are there TAB >> characters in there? the JNIUTIL:: and the return lines have different >> indents in the patch that I'm looking at. That is either sloppy SPC >> character indents, or a TAB is present. >> >> Lastly, there is an extra space character before the ";" in the return >> statement. That should be eliminated. >> >> >> Fix the above three problems, and I'm +1 for you to commit just patch #1. >> >> I have not reviewed #2, but the first patch seems reasonable to >> simplify (as done in #2). I also await others to comment on the >> applicability of patch #2. >> >> I do seem to recall that C++ tried to do away with the preprocessor. >> It would be nice to follow that ideal, but looking at this macro... I >> have no idea how to map it into "proper, non-CPP concepts". If you >> know, that would be better. Otherwise... meh. CPP is just fine with me >> (and screw the C++ academic purity). >> >> Cheers, >> -g >> > >
