Hi Anders,

Good work.

Please find some comments(we have already discussed couple of them)

1) Iam thinking if we can make the **Lend and **Borrow to somehow (optimally) 
be included 
as a part of all service libraries. 
This would mean, the extended length handling(tunneling) is initialized as a 
part of  
sa<svc>Initialize().

This way, the sa**Lend and sa**Borrow that is being proposed here would not be 
necessary
as everything is implicitly handled when that service user calls 
sa<svc>Initialize().

2) I am also thinking whether we should check and see if we can avoid setting 
the environment variable
SA_ENABLE_EXTENDED_NAMES before starting the application.

 I mean, on top of turning on the build option, the additional environment 
variable sounds 
a crude approach! You might have thought/considered this, but still.

3) We had discussed about fixing a upper limit for the extended length. 
Otherwise, it would
be a security vulnerability.


I used the prototype and things did come up. Things also looked to work 
properly.
I will try to write from scratch an application and get back.

Cheers,
Mathi.


>-----Original Message-----
>From: Anders Widell [mailto:anders.wid...@ericsson.com]
>Sent: Friday, May 02, 2014 4:21 PM
>To: Mathivanan Naickan Palanivelu; anders.bjornerst...@ericsson.com
>Cc: opensaf-devel@lists.sourceforge.net
>Subject: [PATCH 0 of 3] Review Request for Extended Name Type [#191]
>
>Summary: Extended Name Type [#191]
>Review request for Trac Ticket(s): 191
>Peer Reviewer(s): Mathi, AndersBj
>Pull request to:
>Affected branch(es): default(4.5)
>Development branch: default
>
>--------------------------------
>Impacted area       Impact y/n
>--------------------------------
> Docs                    y
> Build system            n
> RPM/packaging           n
> Configuration files     n
> Startup scripts         n
> SAF services            y
> OpenSAF services        n
> Core libraries          y
> Samples                 n
> Tests                   n
> Other                   n
>
>
>Comments (indicate scope for each "y" above):
>---------------------------------------------
>These patches add new AIS API functions and definitions for the extended
>SaNameT format, used to tunnel NUL-terminated strings through SaNameT.
>They also add support library functions to be used in agent libraries.
>
>changeset f3cf7ffb53b9a4dd6026cf58c262a46eac8bca3a
>Author:        Anders Widell <anders.wid...@ericsson.com>
>Date:  Fri, 02 May 2014 12:44:08 +0200
>
>       osaf: Add saAisNameLend() and saAisNameBorrow() [#191]
>
>       Add declarations of saAisNameLend() and saAisNameBorrow() to
>saAis.h. Update
>       00-README.conf with description of how to enable the extended
>SaNameT type.
>
>changeset 84def835e0192244ba98eb6c80f192d2db26cd7d
>Author:        Anders Widell <anders.wid...@ericsson.com>
>Date:  Fri, 02 May 2014 12:44:12 +0200
>
>       osaf: Add library functions for handling the extended SaNameT format
>[#191]
>
>       These library functions are primarily intended to be used in agent
>       libraries, to handle the old SAF APIs that still are using the SaNameT
>type.
>
>changeset 99143446a43030c140d0ae95192546108b7639e2
>Author:        Anders Widell <anders.wid...@ericsson.com>
>Date:  Fri, 02 May 2014 12:44:16 +0200
>
>       imm: Add implementations of saAisNameLend and saAisNameBorrow
>[#191]
>
>       The functions saAisNameLend() and saAisNameBorrow() are defined
>in
>       saAis_B_5_14.h, but their implementation is placed in libSaImmOm
>since there
>       is no libSaAis library.
>
>
>Complete diffstat:
>------------------
> 00-README.conf                                     |   18 +++++++++
> osaf/libs/agents/saf/imma/Makefile.am              |    1 +
> osaf/libs/agents/saf/imma/aisa_api.c               |  131
>+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>++++++++
> osaf/libs/core/common/Makefile.am                  |    1 +
> osaf/libs/core/common/include/osaf_extended_name.h |  232
>+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>+
> osaf/libs/core/common/osaf_extended_name.c         |  184
>+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>+++++++++++++++++++++++++++++++++++
> osaf/libs/core/leap/sysf_def.c                     |    3 +
> osaf/libs/saf/include/saAis.h                      |    4 ++
> osaf/libs/saf/include/saAis_B_5_14.h               |   17 ++++++++
> osaf/libs/saf/libSaImm/libSaImmOm.map              |    2 +
> 10 files changed, 593 insertions(+), 0 deletions(-)
>
>
>Testing Commands:
>-----------------
>Build and start OpenSAF.
>
>
>Testing, Expected Results:
>--------------------------
>OpenSAF should build and start successfully.
>
>
>Conditions of Submission:
>-------------------------
>Ack from reviewers.
>
>
>Arch      Built     Started    Linux distro
>-------------------------------------------
>mips        n          n
>mips64      n          n
>x86         n          n
>x86_64      y          y
>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.
>

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to