[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #18 from yongsheng <yongsheng.zhu(a)intel.com> 2010-05-27 03:03:39 PDT ---
> diff --git a/src/sysync/vtimezone.cpp b/src/sysync/vtimezone.cpp
> index 530b97e..60a9232 100644
> --- a/src/sysync/vtimezone.cpp
> +++ b/src/sysync/vtimezone.cpp
> @@ -245,7 +245,11 @@ static bool Get_Bias( string of, string ot, short &bias )
> } // Get_Bias
>
>
> -/*! Fill in the TZ info */
> +/*! Fill in the TZ info.
> + * @return false if some information was found, but couldn't be extracted;
> + * true if not found (in which case c, cBias, cName are unchanged)
> + * or found and extracted
> + */
> static bool GetTZInfo( cAppCharP aText,
> cAppCharP aIdent,
> tChange &c,
> @@ -279,7 +283,6 @@ static bool GetTZInfo( cAppCharP aText,
> // function is called to extract changes for DAYLIGHT. Don't
> // treat this as failure and continue with clean change rules, as
> // before.
> - c = tChange();
> return success;
> }
>
> @@ -295,7 +298,6 @@ static bool GetTZInfo( cAppCharP aText,
> if ( rr.empty() ) {
> // Happens when parsing STANDARD part of VTIMEZONE
> // without DAYLIGHT saving.
> - c = tChange();
> } else if (RRULE2toInternalR ( rr.c_str(), &dtstart, r, aLogP )) {
> // Note: dtstart might have been adjusted by this call in case of DTSTART
> not meeting a occurrence for given RRULE
> string vvv;
> @@ -420,10 +422,14 @@ bool VTIMEZONEtoTZEntry( const char* aText, //
> VTIMEZONE string to be parsed
> t.ident = "";
> t.dynYear= "CUR";
> t.biasDST= 0;
> + t.bias = 0;
> if (!GetTZInfo( aText,VTZ_STD, t.std, t.bias, aStdName, -1, aLogP )) {
> success = false;
> }
> + // default value if not found (which is treated as success by GetTZInfo)
> + dBias= t.bias;
> if (!GetTZInfo( aText,VTZ_DST, t.dst, dBias, aDstName, -1, aLogP )) {
> + // unknown failure, better restore default
> dBias= t.bias;
> success = false;
> }
ok, so please commit your patch.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #17 from pohly <patrick.ohly(a)intel.com> 2010-05-27 02:53:47 PDT ---
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #13)
> > > > No, GetTZInfo() gets dBias as reference and initializes it. It seems that you
> > > > have found a case where it returns "true" without initializing dBias. That case
> > > > needs to be fixed inside GetTZInfo(), it should return false if it cannot find
> > > > dBias.
> > > correct, you're right. Is it possible to assign 'dBias' as a default value
> > > (such as '0') in the beginning of 'GetTZInfo'?
> >
> > That depends - is 0 the right answer for the case where GetTZInfo() currently
> > returns true without setting dBias? It might be that returning false is the
> > better solution.
> Patrick, it's because your newly committed code causes this problem, so I think
> you could help identify my change:
> In synthesis code commit 58f19738bdde67f2a216f4a817deb6a35f731d52,
> 249 static bool GetTZInfo( cAppCharP aText,
> 250 cAppCharP aIdent,
> 251 tChange &c,
> 252 short &cBias,
> 253 string &cName,
> 254 sInt32 aNth, // take nth occurance, -1: take
> last
> 255 TDebugLogger* aLogP )
> 256 {
>
> +
> + a= VStr( aText, aIdent, aNth );
> + if ( a.empty() ) {
> + // Happens for VTIMEZONEs without summer saving time when this
> + // function is called to extract changes for DAYLIGHT. Don't
> + // treat this as failure and continue with clean change rules, as
> + // before.
> + c = tChange();
> + return success;
> + }
> +
> when returning 'success' here, cBias isn't set. So my patch is to set 'cBias'
> as 0 here. Is it correct?
I think that would fail the t.bias == dBias check in VTIMEZONEtoTZEntry().
How about this patch here:
diff --git a/src/sysync/vtimezone.cpp b/src/sysync/vtimezone.cpp
index 530b97e..60a9232 100644
--- a/src/sysync/vtimezone.cpp
+++ b/src/sysync/vtimezone.cpp
@@ -245,7 +245,11 @@ static bool Get_Bias( string of, string ot, short &bias )
} // Get_Bias
-/*! Fill in the TZ info */
+/*! Fill in the TZ info.
+ * @return false if some information was found, but couldn't be extracted;
+ * true if not found (in which case c, cBias, cName are unchanged)
+ * or found and extracted
+ */
static bool GetTZInfo( cAppCharP aText,
cAppCharP aIdent,
tChange &c,
@@ -279,7 +283,6 @@ static bool GetTZInfo( cAppCharP aText,
// function is called to extract changes for DAYLIGHT. Don't
// treat this as failure and continue with clean change rules, as
// before.
- c = tChange();
return success;
}
@@ -295,7 +298,6 @@ static bool GetTZInfo( cAppCharP aText,
if ( rr.empty() ) {
// Happens when parsing STANDARD part of VTIMEZONE
// without DAYLIGHT saving.
- c = tChange();
} else if (RRULE2toInternalR ( rr.c_str(), &dtstart, r, aLogP )) {
// Note: dtstart might have been adjusted by this call in case of DTSTART
not meeting a occurrence for given RRULE
string vvv;
@@ -420,10 +422,14 @@ bool VTIMEZONEtoTZEntry( const char* aText, //
VTIMEZONE string to be parsed
t.ident = "";
t.dynYear= "CUR";
t.biasDST= 0;
+ t.bias = 0;
if (!GetTZInfo( aText,VTZ_STD, t.std, t.bias, aStdName, -1, aLogP )) {
success = false;
}
+ // default value if not found (which is treated as success by GetTZInfo)
+ dBias= t.bias;
if (!GetTZInfo( aText,VTZ_DST, t.dst, dBias, aDstName, -1, aLogP )) {
+ // unknown failure, better restore default
dBias= t.bias;
success = false;
}
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #16 from yongsheng <yongsheng.zhu(a)intel.com> 2010-05-27 01:00:21 PDT ---
(In reply to comment #15)
> (In reply to comment #13)
> > > No, GetTZInfo() gets dBias as reference and initializes it. It seems that you
> > > have found a case where it returns "true" without initializing dBias. That case
> > > needs to be fixed inside GetTZInfo(), it should return false if it cannot find
> > > dBias.
> > correct, you're right. Is it possible to assign 'dBias' as a default value
> > (such as '0') in the beginning of 'GetTZInfo'?
>
> That depends - is 0 the right answer for the case where GetTZInfo() currently
> returns true without setting dBias? It might be that returning false is the
> better solution.
Patrick, it's because your newly committed code causes this problem, so I think
you could help identify my change:
In synthesis code commit 58f19738bdde67f2a216f4a817deb6a35f731d52,
249 static bool GetTZInfo( cAppCharP aText,
250 cAppCharP aIdent,
251 tChange &c,
252 short &cBias,
253 string &cName,
254 sInt32 aNth, // take nth occurance, -1: take
last
255 TDebugLogger* aLogP )
256 {
+
+ a= VStr( aText, aIdent, aNth );
+ if ( a.empty() ) {
+ // Happens for VTIMEZONEs without summer saving time when this
+ // function is called to extract changes for DAYLIGHT. Don't
+ // treat this as failure and continue with clean change rules, as
+ // before.
+ c = tChange();
+ return success;
+ }
+
when returning 'success' here, cBias isn't set. So my patch is to set 'cBias'
as 0 here. Is it correct?
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #15 from pohly <patrick.ohly(a)intel.com> 2010-05-26 23:50:59 PDT ---
(In reply to comment #13)
> > No, GetTZInfo() gets dBias as reference and initializes it. It seems that you
> > have found a case where it returns "true" without initializing dBias. That case
> > needs to be fixed inside GetTZInfo(), it should return false if it cannot find
> > dBias.
> correct, you're right. Is it possible to assign 'dBias' as a default value
> (such as '0') in the beginning of 'GetTZInfo'?
That depends - is 0 the right answer for the case where GetTZInfo() currently
returns true without setting dBias? It might be that returning false is the
better solution.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #14 from pohly <patrick.ohly(a)intel.com> 2010-05-26 23:49:35 PDT ---
(In reply to comment #12)
> > What fix do you recommend? Initialize m_flag2 outside of the if/else branches?
> Yes, but i'm not very sure whether it will affect the engine.
The worst thing that can happen is that something is initialized which is never
read again. Unless initialization is very expensive, this isn't a problem.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #13 from yongsheng <yongsheng.zhu(a)intel.com> 2010-05-26 20:24:35 PDT ---
> No, GetTZInfo() gets dBias as reference and initializes it. It seems that you
> have found a case where it returns "true" without initializing dBias. That case
> needs to be fixed inside GetTZInfo(), it should return false if it cannot find
> dBias.
correct, you're right. Is it possible to assign 'dBias' as a default value
(such as '0') in the beginning of 'GetTZInfo'?
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #11 from yongsheng <yongsheng.zhu(a)intel.com> 2010-05-26 20:14:37 PDT ---
> I agree with your root cause analysis, but not with this solution. I believe it
> leads to accessing the cost value at index #-2, which falls of the lower end of
> the array.
I don't think so. for i and j will never equal 0 when calculating this
expression.
please see lcs.h:145.
145 if (i == 0 || j == 0) {
146 sub[i][j].choice = NONE;
147 sub[i][j].length = 0;
148 sub[i][j].cost = 0;
149 } else if (access.entry_at(a, i - 1) == access.entry_at(b, j -
1)) {
> IMHO the cost function was called with correct indices. Basically both one
> before array and one after the end are valid and should give the same result as
> the indices just inside the array.
I don't quite understand what the 'cost' is. but here, based on your
description, the first cost should be equal to the cost of second element
substract to that of the first element.
> The implementation only handled index -1, but not index == size(). It also did
> not work for empty arrays.
> Here's a more complete solution, committed to branch mbc1007_pohly. Yongsheng,
> do you agree?
ok, please do.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #10 from pohly <patrick.ohly(a)intel.com> 2010-05-26 08:43:31 PDT ---
(In reply to comment #4)
> There are many cases reporting 'conditional jump or move depends on
> uninitialised value(s)' in synthesis library. The reason is to read a variable
> value without initialization. For the detail valgrind output, please see
> http://zys.sh.intel.com/valgrind-synthesis-conditional.txt
>
> synthesis library has many cases of this kind. A typical case is like this:
> class base {
> public:
> virtual bool xxx() { return false; }
> };
> class A : public {
> bool m_flag1;
> bool m_flag2;
> public:
> A()
> {
> if( IS_CLIENT) { // current engine is a client
> #ifdef SYSYNC_CLIENT
> m_flag1 = true;
> #endif
> } else {
> #ifdef SYSYNC_SERVER
> m_flag2 = true;
> #endif
> }
> }
> #ifdef SYSYNC_SERVER
> bool xxx() { return m_flag2; }
> #endif
> };
>
> void errorFunc(A* a) {
> if(a->xxx()) {
> // do sth
> }
> }
>
> This case will report an the above error. The root cause is because we enable
> sync client and server at the same time. When we create A object in client
> engine, then errorFunc will actually call A::xxx() instead of base::xxx().
> Because A constructor doesn't initialise m_flag2 in client mode, this kind of
> error occurs. Please see libsynthesis/src/sysync/syncagent.cpp:635.
> Many of errors are due to this kind of code. I don't know whether there are
> other places in synthesis. but i think possbily.
What fix do you recommend? Initialize m_flag2 outside of the if/else branches?
I assume m_flag2 maps to fUseRespURI in the example above?
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months
[Bug 1007] nightly testing + valgrind results
by bugzilla＠meego.com
http://bugs.meego.com/show_bug.cgi?id=1007
--- Comment #9 from pohly <patrick.ohly(a)intel.com> 2010-05-26 08:31:06 PDT ---
(In reply to comment #7)
> In vtimezone.cpp, valgrind issue is at line 431: dBias is not initilised.
> 416 short dBias; // the full bias for DST
> 417 bool success = true;
> 418
> 419 t.name = "";
> 420 t.ident = "";
> 421 t.dynYear= "CUR";
> 422 t.biasDST= 0;
> 423 if (!GetTZInfo( aText,VTZ_STD, t.std, t.bias, aStdName, -1, aLogP )) {
> 424 success = false;
> 425 }
> 426 if (!GetTZInfo( aText,VTZ_DST, t.dst, dBias, aDstName, -1, aLogP )) {
> 427 dBias= t.bias;
> 428 success = false;
> 429 }
> 430
> 431 if (t.bias == dBias) ClrDST( t ); // no DST ?
> This auto dBias never be set. I don't know the reason. It seems to me that the
> logic here is strange. 'dBias' is set if and only if 426 is true.
No, GetTZInfo() gets dBias as reference and initializes it. It seems that you
have found a case where it returns "true" without initializing dBias. That case
needs to be fixed inside GetTZInfo(), it should return false if it cannot find
dBias.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11 years, 12 months