Re: [fossil-users] TH1: set v 0; unset v; info exists v;
Sergei Gavrikov wrote: As I could see Th_ExistsVar() does miss a test for pValue-zData as Th_GetVar() does http://fossil-scm.org/index.html/artifact/a561c58c237b3eb43eaf55e6f9cc6a9b8a26e5d1?ln=1142-1149. It looks like the same check is needed for Th_UnsetVar() too. # Expect {TH_ERROR: no such variable: i}, but got {i} % fossil test-th-eval 'set i 0; unset i; unset i' i Sergei Index: src/th.c == --- src/th.c +++ src/th.c @@ -1251,10 +1251,14 @@ Th_Variable *pValue; pValue = thFindValue(interp, zVar, nVar, 0, 1, 0); if( !pValue ){ return TH_ERROR; + } + if( !pValue-zData ){ +Th_ErrorMessage(interp, no such variable:, zVar, nVar); +return TH_ERROR; } Th_Free(interp, pValue-zData); pValue-zData = 0; if( pValue-pHash ){ Index: test/th1.test == --- test/th1.test +++ test/th1.test @@ -147,5 +147,10 @@ ### fossil test-th-eval unset var test th1-unset-2 {$RESULT eq {TH_ERROR: no such variable: var}} + +### + +fossil test-th-eval set var 1; unset var; unset var +test th1-unset-3 {$RESULT eq {TH_ERROR: no such variable: var}} ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Re: [fossil-users] TH1: set v 0; unset v; info exists v;
On Mon, 13 Jan 2014, Joe Mistachkin wrote: Sergei Gavrikov wrote: It looks like the same check is needed for Th_UnsetVar() too. Actually, the issue with Th_UnsetVar was slightly different; however, it should be fixed now: http://www.fossil-scm.org/index.html/info/1ebe4b02e4 Really. Thank you for your look on this. But, please, try comment-out http://fossil-scm.org/index.html/artifact/4d39579b42eb0d11?ln=381-389 I have got a few assertion fails in th1.test (and the last test failed). If to have that assert is bad idea, then at the least we have to have fossil_warning() there. Sergei ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Re: [fossil-users] TH1: set v 0; unset v; info exists v;
On Sun, 5 Jan 2014, Joe Mistachkin wrote: Sergei Gavrikov wrote: It seems this was introduced with Th_ExistsVar() http://fossil-scm.org/index.html/artifact/a561c58c237b3eb43eaf55e6f9cc6a9b8a 26e5d1?ln=1154-1159 (check-in http://fossil-scm.org/index.html/info/4f8c8975bc). As I could see Th_ExistsVar() does miss a test for pValue-zData as Th_GetVar() does http://fossil-scm.org/index.html/artifact/a561c58c237b3eb43eaf55e6f9cc6a9b8a 26e5d1?ln=1142-1149. Right? Could you, please, fix that? Sergei Gavrikov also wrote: Th_UnsetVar() creates (creatok=1), then unset variable. So, unset never raise an error. It is okay? Thanks for the report(s). Fixed here: https://www.fossil-scm.org/index.html/info/7164f52baa And here: https://www.fossil-scm.org/index.html/info/99bdfa0b95 Thank you for the fixes! I'm sorry, but, I found yet another issue with Th_ExistsVar(). If a variable is not exists, Th_ExistsVar() does clear TH stack trace: # Expect {foo}, but get nothing {}. catch foo; info exists bar; set ::th_stack_trace Call path: Th_ExistsVar() - thFindValue() - Th_ErrorMessage() - ... Th_SetVar(interp, (char *)::th_stack_trace, -1, 0, 0); Unfortunately, there is no way to fix this without changing an interface function thFindVal(). As I could see thFindValue() cannot silent return 0 (not found, no care) it always calls Th_ErrorMeesage() on fails. I tested some workaround, it does not change default behavior and other Th_*Var() are happy, please, look on a draft patch (original comments have not been fixed). Could we expand 'arrayok' flag to a bitfield flag or that is ugly thing? Thank you for your time. Sergei Index: src/th.c == --- src/th.c +++ src/th.c @@ -1042,10 +1042,16 @@ *pnInner = nInner; *pisGlobal = isGlobal; return TH_OK; } +#define FV_ArrayOk0x0001 +#define FV_NoError0x8000 + +#define FIND_ARRAYOK 0x0001 +#define FIND_NOERROR 0x8000 + /* ** Input string (zVar, nVar) contains a variable name. This function locates ** the Th_Variable structure associated with the named variable. The ** variable name may be a global or local scalar or array variable ** @@ -1060,11 +1066,11 @@ static Th_Variable *thFindValue( Th_Interp *interp, const char *zVar, /* Pointer to variable name */ int nVar, /* Number of bytes at nVar */ int create,/* If true, create the variable if not found */ - int arrayok/* If true, an array is Ok. Otherwise array==error */ + int flags ){ const char *zOuter; int nOuter; const char *zInner; int nInner; @@ -1114,20 +1120,22 @@ pValue = Th_Malloc(interp, sizeof(Th_Variable)); pValue-nRef = 1; pEntry-pData = (void *)pValue; } }else{ -if( pValue-pHash !arrayok ){ +if( pValue-pHash (flagsFV_ArrayOk)!=FIND_ARRAYOK ){ Th_ErrorMessage(interp, variable is an array:, zOuter, nOuter); return 0; } } return pValue; no_such_var: - Th_ErrorMessage(interp, no such variable:, zVar, nVar); + if( (flagsFV_NoError)!=FIND_NOERROR ){ +Th_ErrorMessage(interp, no such variable:, zVar, nVar); + } return 0; } /* ** String (zVar, nVar) must contain the name of a scalar variable or @@ -1154,11 +1162,11 @@ /* ** Return true if variable (zVar, nVar) exists. */ int Th_ExistsVar(Th_Interp *interp, const char *zVar, int nVar){ - Th_Variable *pValue = thFindValue(interp, zVar, nVar, 0, 0); + Th_Variable *pValue = thFindValue(interp, zVar, nVar, 0, FIND_NOERROR); return pValue pValue-zData; } /* ** String (zVar, nVar) must contain the name of a scalar variable or Index: test/th1.test == --- test/th1.test +++ test/th1.test @@ -132,10 +132,15 @@ ### fossil test-th-eval set var 1; unset var; expr {\$var+0} test th1-info-exists-5 {$RESULT eq {TH_ERROR: no such variable: var}} + +### + +fossil test-th-eval catch foo; info exists bar; set ::th_stack_trace +test th1-info-exists-6 {$RESULT eq {foo}} ### fossil test-th-eval set var 1; unset var test th1-unset-1 {$RESULT eq {var}} ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Re: [fossil-users] TH1: set v 0; unset v; info exists v;
Sergei Gavrikov wrote: Thank you for the fixes! I'm sorry, but, I found yet another issue with Th_ExistsVar(). If a variable is not exists, Th_ExistsVar() does clear TH stack trace: Thanks again, fixed here: https://www.fossil-scm.org/index.html/info/9765b03759 -- Joe Mistachkin ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Re: [fossil-users] TH1: set v 0; unset v; info exists v;
On Thu, 9 Jan 2014, Joe Mistachkin wrote: Sergei Gavrikov wrote: Thank you for the fixes! I'm sorry, but, I found yet another issue with Th_ExistsVar(). If a variable is not exists, Th_ExistsVar() does clear TH stack trace: Thanks again, fixed here: https://www.fossil-scm.org/index.html/info/9765b03759 Thank you! Sergei ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Re: [fossil-users] TH1: set v 0; unset v; info exists v;
On Sun, 5 Jan 2014, Sergei Gavrikov wrote: [no lyrics] % ./fossil version This is fossil version 1.28 [da90bbe591] 2014-01-04 16:17:47 UTC % ./fossil test-th-eval 'set v 1; unset v; info exists v' 1 We expect 0. No issues with unset % ./fossil test-th-eval 'set v 1; unset v; expr $v+0' TH_ERROR: no such variable: v Hm, unset % ./fossil test-th-eval 'unset absent' absent Th_UnsetVar() creates (creatok=1), then unset variable. So, unset never raise an error. It is okay? With patch below TH1 behaves like TCL % ./fossil test-th-eval 'unset absent' TH_ERROR: no such variable: absent % ./fossil test-th-eval 'set v 1; unset v; info exists v' 0 Sergei --- src/th.c +++ src/th.c @@ -1154,11 +1154,12 @@ /* ** Return true if variable (zVar, nVar) exists. */ int Th_ExistsVar(Th_Interp *interp, const char *zVar, int nVar){ - return thFindValue(interp, zVar, nVar, 0, 0)!=0; + Th_Variable *pValue = thFindValue(interp, zVar, nVar, 0, 0); + return pValue!=0 pValue-zData!=0; } /* ** String (zVar, nVar) must contain the name of a scalar variable or ** array member. If the variable does not exist it is created. The @@ -1240,11 +1241,11 @@ ** in the interpreter result and TH_ERROR is returned. */ int Th_UnsetVar(Th_Interp *interp, const char *zVar, int nVar){ Th_Variable *pValue; - pValue = thFindValue(interp, zVar, nVar, 1, 1); + pValue = thFindValue(interp, zVar, nVar, 0, 1); if( !pValue ){ return TH_ERROR; } Th_Free(interp, pValue-zData); ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Re: [fossil-users] TH1: set v 0; unset v; info exists v;
Sergei Gavrikov wrote: It seems this was introduced with Th_ExistsVar() http://fossil-scm.org/index.html/artifact/a561c58c237b3eb43eaf55e6f9cc6a9b8a 26e5d1?ln=1154-1159 (check-in http://fossil-scm.org/index.html/info/4f8c8975bc). As I could see Th_ExistsVar() does miss a test for pValue-zData as Th_GetVar() does http://fossil-scm.org/index.html/artifact/a561c58c237b3eb43eaf55e6f9cc6a9b8a 26e5d1?ln=1142-1149. Right? Could you, please, fix that? Sergei Gavrikov also wrote: Th_UnsetVar() creates (creatok=1), then unset variable. So, unset never raise an error. It is okay? Thanks for the report(s). Fixed here: https://www.fossil-scm.org/index.html/info/7164f52baa And here: https://www.fossil-scm.org/index.html/info/99bdfa0b95 -- Joe Mistachkin ___ fossil-users mailing list fossil-users@lists.fossil-scm.org http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users