Hi Vu,
yes, reinterpret_cast should be avoided if possible, in this case
static_cast<const char*> is better. You can ignore my comment
about passing `attrDefaultValueBuffer` directly, it is not valid,.
/Thanks HansN
-----Original Message-----
From: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
Sent: den 7 augusti 2018 05:33
To: Hans Nordebäck <hans.nordeb...@ericsson.com>; Lennart Lund
<lennart.l...@ericsson.com>; Gary Lee <gary....@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [PATCH 1/1] imm: attrDefaultValue is set to NULL if no default
value is given [#2901]
Hi Hans,
Thanks for your comments. I will update the code using static_cast<const char*>.
Passing `attrDefaultValueBuffer` directly to strlen() without type-casting will
generate a compile error because of invalid conversion from `void*` to `const
char*`, I think.
Regards, Vu
> -----Original Message-----
> From: Hans Nordeback <hans.nordeb...@ericsson.com>
> Sent: Monday, August 6, 2018 7:07 PM
> To: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>;
> lennart.l...@ericsson.com; gary....@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] imm: attrDefaultValue is set to NULL if no
> default value is given [#2901]
>
> Hi Vu,
>
> ack, review only. Minor comment below./Thanks HansN
>
>
> On 07/30/2018 10:46 AM, Vu Minh Nguyen wrote:
> > When explicitly having <default-value> tag, but no value is given:
> > <default-value></default-value>, set NULL to attrDefaultValue.
> > ---
> > src/imm/immloadd/imm_loader.cc | 3 ++-
> > src/imm/tools/imm_import.cc | 3 ++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/imm/immloadd/imm_loader.cc
> b/src/imm/immloadd/imm_loader.cc
> > index de5a575e9..ad9785e92 100644
> > --- a/src/imm/immloadd/imm_loader.cc
> > +++ b/src/imm/immloadd/imm_loader.cc
> > @@ -1909,7 +1909,8 @@ void addClassAttributeDefinition(
> > attrDefinition.attrFlags = attrFlags;
> >
> > /* Set the default value */
> > - if (attrDefaultValueBuffer) {
> > + if (attrDefaultValueBuffer &&
> [HansN] use static_cast<const char*>(attrDefaultValueBuffer) instead
> or
>
> (strlen(attrDefaultValueBuffer) > 0)) { (instead of the reinterpret_cast,
> not
> needed though)
>
> > + (strlen(reinterpret_cast<char*>(attrDefaultValueBuffer)) >
> > + 0)) {
> > charsToValueHelper(&attrDefinition.attrDefaultValue, attrValueType,
> > (const char *)attrDefaultValueBuffer);
> > } else {
> > diff --git a/src/imm/tools/imm_import.cc
> > b/src/imm/tools/imm_import.cc index e2bdcba5c..8145ec572 100644
> > --- a/src/imm/tools/imm_import.cc
> > +++ b/src/imm/tools/imm_import.cc
> > @@ -2444,7 +2444,8 @@ static void
> addClassAttributeDefinition(ParserState *state) {
> > }
> >
> > /* Set the default value */
> > - if (state->attrDefaultValueSet) {
> > + if (state->attrDefaultValueSet &&
> [HansN] use static_cast<const char*>(attrDefaultValueBuffer) instead
> or
>
> (strlen(attrDefaultValueBuffer) > 0)) { (instead of the reinterpret_cast,
> not
> needed though)
>
> > +
> > + (strlen(reinterpret_cast<char*>(state->attrDefaultValueBuffer)) >
> > + 0)) {
> > if (charsToValueHelper(&attrDefinition.attrDefaultValue,
> > state->attrValueType,
> > state->attrDefaultValueBuffer,
> > state->strictParse)) {
------------------------------------------------------------------------------
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
