Hi! I have started looking at this now. Two comments so far:
1) In NtfFilter::cmpSaNameT(), do you really have to duplicate the strings? I think it should be fine to just get pointers to the strings using saAisNameBorrow() and then compare the original pointers. The function decodeSaNameT() guarantees that the SaNameT will have a terminating NUL character, so the original pointers should be safe to use at least if they were produced by this function. 2) Shouldn't the ntfsv_filter_header_free() function loop through notificationObjects and notifyingObjects and call osaf_extended_name_free() on each name in them? / Anders Widell On 07/15/2014 08:16 AM, Minh Hon Chau wrote: > Summary: NTF: Support DNs longer than 255 bytes [#873] v3 > Review request for Trac Ticket(s): [#873] > Peer Reviewer(s): AndersW, HansF, Praveen, Mathi, Zoran, AndersBj > Pull request to: > Affected branch(es): default > Development branch: default > > -------------------------------- > Impacted area Impact y/n > -------------------------------- > Docs n > Build system n > RPM/packaging n > Configuration files n > Startup scripts n > SAF services y > OpenSAF services n > Core libraries n > Samples n > Tests n > Other n > > > Comments (indicate scope for each "y" above): > --------------------------------------------- > Patch series version 3: > - Fix endian issue in common library > - Not allow objects having length greater than kMaxDnLength > in ntfread, ntfsend. > - Add testcase for AdditionalInfo with extended SaNameT > > > changeset 41e6acf50d610229b7dbf1590a5d98d9d2e80dae > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > NTF: Adapt NTF API to support long DNs [#873] > > changeset c6092d94fae5c0db5b98e91bc68d0d58dc066c31 > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > NTF: Adapt NTF common library to support long DNs [#873] v3 > > (1)Fix endian issue in ntfs_sanamet_length, ntfsv_sanamet_alloc, > ntfsv_sanamet_steal > > changeset d04dcc662b9b498701764557169ce5fb13eb6930 > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > NTF: Adapt NTF osaf service to support long DNs [#873] > > changeset 019e5387c457294db13b1a2ccbb0eb28f6b9cb62 > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > NTF: Adapt NTFIMCND to support long DNs [#873] > > changeset 97fd2415132ed2e3d934774d00294e2c81754204 > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > NTF: Adapt NTF tools (ntfread, ntfsend, ntfsubscribe) to support long > DNs > [#873] v2 > > (1) Not allow objects specified in command having string greater than > kMaxDnLength > > changeset 6f31240a26d0c3757a6f13da82cff9d9244f2325 > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > NTF: Add ntftest test cases for notification with long dn objects > [#873] v2 > > (1) Add testcase for AdditionalInfo with extended SaNameT > > changeset 44c410176a04b5cd260f8914da117386c740b461 > Author: Minh Hon Chau <[email protected]> > Date: Tue, 15 Jul 2014 15:50:16 +1000 > > Temporary patch to solve compilation dependency > > This patch is not part of #873, it's made to solve the dependency since > NTF > is using immutil.c and saflog.c. NOTE that it will not be pushed > together > with #873 > > > Added Files: > ------------ > tests/ntfsv/tet_longDnObject_notification.c > > > Complete diffstat: > ------------------ > osaf/libs/agents/saf/ntfa/Makefile.am | 1 + > osaf/libs/agents/saf/ntfa/ntfa_api.c | 25 +- > osaf/libs/common/ntfsv/Makefile.am | 1 + > osaf/libs/common/ntfsv/include/ntfsv_mem.h | 7 + > osaf/libs/common/ntfsv/ntfsv_enc_dec.c | 37 ++- > osaf/libs/common/ntfsv/ntfsv_mem.c | 151 +++++++++++- > osaf/libs/saf/libSaNtf/Makefile.am | 1 + > osaf/services/saf/ntfsv/ntfimcnd/Makefile.am | 2 + > osaf/services/saf/ntfsv/ntfimcnd/ntfimcn_imm.c | 91 ++++--- > osaf/services/saf/ntfsv/ntfimcnd/ntfimcn_notifier.c | 54 ++-- > osaf/services/saf/ntfsv/ntfs/Makefile.am | 1 + > osaf/services/saf/ntfsv/ntfs/NtfFilter.cc | 34 +- > osaf/services/saf/ntfsv/ntfs/NtfLogger.cc | 5 +- > osaf/services/saf/ntfsv/ntfs/ntfs_evt.c | 5 +- > osaf/services/saf/ntfsv/ntfs/ntfs_main.c | 5 + > osaf/tools/safimm/src/immutil.c | 57 ++-- > osaf/tools/saflog/src/saflog.c | 10 +- > osaf/tools/safntf/include/ntfclient.h | 3 +- > osaf/tools/safntf/ntfread/Makefile.am | 6 +- > osaf/tools/safntf/ntfread/ntfread.c | 23 +- > osaf/tools/safntf/ntfsend/Makefile.am | 6 +- > osaf/tools/safntf/ntfsend/ntfsend.c | 29 +- > osaf/tools/safntf/ntfsubscribe/Makefile.am | 6 +- > osaf/tools/safntf/ntfsubscribe/ntfsubscribe.c | 4 + > osaf/tools/safntf/src/Makefile.am | 1 + > osaf/tools/safntf/src/ntfclient.c | 58 ++-- > tests/ntfsv/Makefile.am | 3 +- > tests/ntfsv/tet_longDnObject_notification.c | 957 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/unit_test_fw/inc/util.h | 5 +- > tests/unit_test_fw/src/Makefile.am | 1 + > tests/unit_test_fw/src/util.c | 14 +- > 31 files changed, 1372 insertions(+), 231 deletions(-) > > > Testing Commands: > ----------------- > ntftest > > > Testing, Expected Results: > -------------------------- > All tests pass (plus testsuite 35) > > > Conditions of Submission: > ------------------------- > <<HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC>> > > > Arch Built Started Linux distro > ------------------------------------------- > mips n n > mips64 n n > x86 n n > x86_64 y n > powerpc n n > powerpc64 n n > > > Reviewer Checklist: > ------------------- > [Submitters: make sure that your review doesn't trigger any checkmarks!] > > > Your checkin has not passed review because (see checked entries): > > ___ Your RR template is generally incomplete; it has too many blank entries > that need proper data filled in. > > ___ You have failed to nominate the proper persons for review and push. > > ___ Your patches do not have proper short+long header > > ___ You have grammar/spelling in your header that is unacceptable. > > ___ You have exceeded a sensible line length in your headers/comments/text. > > ___ You have failed to put in a proper Trac Ticket # into your commits. > > ___ You have incorrectly put/left internal data in your comments/files > (i.e. internal bug tracking tool IDs, product names etc) > > ___ You have not given any evidence of testing beyond basic build tests. > Demonstrate some level of runtime or other sanity testing. > > ___ You have ^M present in some of your files. These have to be removed. > > ___ You have needlessly changed whitespace or added whitespace crimes > like trailing spaces, or spaces before tabs. > > ___ You have mixed real technical changes with whitespace and other > cosmetic code cleanup changes. These have to be separate commits. > > ___ You need to refactor your submission into logical chunks; there is > too much content into a single commit. > > ___ You have extraneous garbage in your review (merge commits etc) > > ___ You have giant attachments which should never have been sent; > Instead you should place your content in a public tree to be pulled. > > ___ You have too many commits attached to an e-mail; resend as threaded > commits, or place in a public tree for a pull. > > ___ You have resent this content multiple times without a clear indication > of what has changed between each re-send. > > ___ You have failed to adequately and individually address all of the > comments and change requests that were proposed in the initial review. > > ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) > > ___ Your computer have a badly configured date and time; confusing the > the threaded patch review. > > ___ Your changes affect IPC mechanism, and you don't present any results > for in-service upgradability test. > > ___ Your changes affect user manual and documentation, your patch series > do not contain the patch that updates the Doxygen manual. > ------------------------------------------------------------------------------ Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
