Hi Lennart,
Just want to make clear that this kind of ridiculous bug did not exist
before introducing #1398. It used to work perfectly before that.
Regards, Vu
> -----Original Message-----
> From: Lennart Lund <lennart.l...@ericsson.com>
> Sent: Tuesday, July 31, 2018 3:04 PM
> To: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>; Gary Lee
> <gary....@dektech.com.au>
> Cc: opensaf-devel@lists.sourceforge.net; Lennart Lund
> <lennart.l...@ericsson.com>
> Subject: RE: [PATCH 0/1] Review Request for smf: fix numberic attribute
> types not accept boolean values [#2902]
>
> Hi Vu
>
> STOP!
> I have done some more investigations around this and it seems as if this
fix
> shall not be done at all!
> I have looked in the specifications (AIS and PR) and cannot find anything
> saying that Booleans should be handled according to this ticket. I have
also
> checked old code before #1398 where the first SMF refactoring using immccb
> was done and cannot find any code converting "true"/"false" to numeric IMM
> values for Boolean. I have also talked to Jonas and Faisal and Faisal was
sure
> of that this has never been handled like this.
> My conclusion is that this ticket is most likely invalid and this fix is
NBC. So DO
> NOT PUSH! for now. However I will do some more investigations.
>
> If and only if any fixes at all should be done:
> Do the fixes in the SMF code. See my suggestion for how to do that in my
> previous reply.
> If the SMF way to interpret the strings "true" and "false" is an SMF thing
not
> something that has to do with the information model The immccb is a
"pure"
> IMM CCB handler and shall not be seen as a part of SMF and shall stay that
> way. See immccb as something that belongs to "src/base" and that could be
> used in more services than SMF. There is no reason this fix cannot be done
> just as easy in the SMF code and since this is SMF handling of the strings
> "true" and "false".
> To implement this fix in immccb should be seen the same way as if an IMM
> ticket was written and it was implemented in the IMM service! That would
be
> ridiculous.
>
> See also my comments below.
>
> Thanks
> Lennart
>
> > -----Original Message-----
> > From: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
> > Sent: den 31 juli 2018 03:53
> > To: Lennart Lund <lennart.l...@ericsson.com>
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: RE: [PATCH 0/1] Review Request for smf: fix numberic attribute
> > types not accept boolean values [#2902]
> >
> > Hi Lennart,
> >
> > You are right that IMM model does not support Boolean type, but what we
> > were
> > talking here is about Boolean value.
> [Lennart] Yes but this is an SMF definition and has nothing to do with IMM
so
> no IMM handler should handle this
> >
> > In the C programming language, `true`/`false` boolean value are macro
that
> > would expand to 1 and 0 respectively.
> > Therefore, it should be valid to assign these values to numeric data
types
> > and should be located in immccb, I think.
> [Lennart] I do not agree. We are not talking about the any programming
> language but the IMM model and campaign files
> >
> > int64_values.attribute_name = "SaInt64TValues";
> > int64_values.value_type = SA_IMM_ATTR_SAINT64T;
> > int64_values.AddValue(std::to_string(1)); // Should be the same as
> > int64_values.AddValue("true");
> [Lennart] No. "1" != "true" and "true" is not a numeric value. The immccb
> does not and shall not have any dependencies to SMF
> >
> > Regards, Vu
> >
> > > -----Original Message-----
> > > From: Lennart Lund <lennart.l...@ericsson.com>
> > > Sent: Monday, July 30, 2018 9:55 PM
> > > To: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
> > > Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> > > <vu.m.ngu...@dektech.com.au>; Lennart Lund
> > > <lennart.l...@ericsson.com>
> > > Subject: RE: [PATCH 0/1] Review Request for smf: fix numberic
attribute
> > > types not accept boolean values [#2902]
> > >
> > > Hi Vu
> > >
> > > The fix is done in the wrong place. The model modifier (immccb)
handles
> > IMM
> > > according to IMM rules and Boolean does not exist there. Converting
text
> > > strings "true" and "false" to numeric 1 and 0 is an SMF special case.
The
> > > model modifier shall be kept completely independent of SMF.
> > > The fix shall be done in the SMF code instead and can be done when
data
> > for
> > > a CCB is stored.
> > > It could for example be done in the SmfImmAttribute class (see file
> > > SmfImmOperation.h):
> > > If m_type (set using method SetAttributeType()) is a numeric type and
> > data
> > is
> > > "true" or "false" then in method AddAttributeValue() the conversion to
"1"
> > or
> > > "0" can be done.
> > >
> > > Thanks
> > > Lennart
> > >
> > > > -----Original Message-----
> > > > From: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
> > > > Sent: den 30 juli 2018 10:38
> > > > To: Lennart Lund <lennart.l...@ericsson.com>
> > > > Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
> > > > <vu.m.ngu...@dektech.com.au>
> > > > Subject: [PATCH 0/1] Review Request for smf: fix numberic attribute
> > types
> > > > not accept boolean values [#2902]
> > > >
> > > > Summary: smf: fix numberic attribute types not accept boolean values
> > > > [#2902]
> > > > Review request for Ticket(s): 2902
> > > > Peer Reviewer(s): Lennart
> > > > Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
> > > > Affected branch(es): develop
> > > > Development branch: ticket-2902
> > > > Base revision: ede5191f9caf41836a65acaffd648e7ac0b00590
> > > > Personal repository: git://git.code.sf.net/u/winhvu/review
> > > >
> > > > --------------------------------
> > > > 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):
> > > > ---------------------------------------------
> > > > *** EXPLAIN/COMMENT THE PATCH SERIES HERE ***
> > > >
> > > > revision 2d2cf2d1bc91e3204c614cb37e1edbf461b2b240
> > > > Author: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
> > > > Date: Mon, 30 Jul 2018 15:23:54 +0700
> > > >
> > > > smf: fix numberic attribute types not accept boolean values [#2902]
> > > >
> > > > This patch ensures that giving boolean value to numeric attribute
> > > > types must be accepted.
> > > >
> > > >
> > > >
> > > > Complete diffstat:
> > > > ------------------
> > > > src/smf/smfd/imm_modify_config/attribute.cc | 20 ++++++++++++++--
> -
> > ---
> > > > src/smf/smfd/imm_modify_demo/test_ccbhdl.cc | 12 +++++++++++-
> > > > 2 files changed, 25 insertions(+), 7 deletions(-)
> > > >
> > > >
> > > > Testing Commands:
> > > > -----------------
> > > > Run test_ccbhdl
> > > >
> > > >
> > > > Testing, Expected Results:
> > > > --------------------------
> > > > No failure
> > > >
> > > >
> > > > Conditions of Submission:
> > > > -------------------------
> > > > Ack from Lennart
> > > >
> > > >
> > > > Arch Built Started Linux distro
> > > > -------------------------------------------
> > > > mips n n
> > > > mips64 n n
> > > > x86 n n
> > > > x86_64 n 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 ~/.gitconfig file (i.e. user.name,
> > user.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.
> >
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
