Re: [Monetdb-developers] [Monetdb-sql-checkins] sql/src/server sql_schema.mx, 1.122, 1.123
Stefan Manegold wrote:
Update of /cvsroot/monetdb/sql/src/server In directory sc8-pr-cvs16:/tmp/cvs-serv14592/src/server
Modified Files: sql_schema.mx Log Message:
[since I spent (almost) all night and morning on compiling & testing, anyway:]
for the umpteenth time: make SQL compile with icc, again, by removind set but unused variable --- some more concentration during coding might save us quite some time ....
Romulo,
could you please check, whether the code still complies with you intention?
Index: sql_schema.mx
RCS file: /cvsroot/monetdb/sql/src/server/sql_schema.mx,v retrieving revision 1.122 retrieving revision 1.123 diff -u -d -r1.122 -r1.123 --- sql_schema.mx 17 Apr 2007 12:27:07 -0000 1.122 +++ sql_schema.mx 18 Apr 2007 09:16:26 -0000 1.123 @@ -602,11 +602,10 @@ { char *tname = qname_table(qname); sql_schema *ss = cur_schema(sql);
sql_trigger * t= NULL;
if (!schema_privs(sql->role_id, ss)) return sql_error(sql, 02, "DROP TRIGGER: access denied for %s to schema ;'%s'", stack_get_string(sql, "current_user"), ss->base.name);
if ((t = mvc_bind_trigger(sql, ss, tname )) == NULL)
- if (mvc_bind_trigger(sql, ss, tname ) == NULL)
Why I should not use the t? I found similar examples in the code. Maybe the the t was used after. I agree with this change I think it does affect the semantics of the function.
Regards, Romulo
return sql_error(sql, 02, "DROP TRIGGER: unknown trigger %s\n", tname);mvc_drop_trigger(sql, ss, tname); return stmt_none();
This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Monetdb-sql-checkins mailing list Monetdb-sql-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-sql-checkins
Romulo Goncalves wrote:
Stefan Manegold wrote:
Update of /cvsroot/monetdb/sql/src/server In directory sc8-pr-cvs16:/tmp/cvs-serv14592/src/server
Modified Files: sql_schema.mx Log Message:
[since I spent (almost) all night and morning on compiling & testing, anyway:]
for the umpteenth time: make SQL compile with icc, again, by removind set but unused variable --- some more concentration during coding might save us quite some time ....
Romulo,
could you please check, whether the code still complies with you intention?
Index: sql_schema.mx
RCS file: /cvsroot/monetdb/sql/src/server/sql_schema.mx,v retrieving revision 1.122 retrieving revision 1.123 diff -u -d -r1.122 -r1.123 --- sql_schema.mx 17 Apr 2007 12:27:07 -0000 1.122 +++ sql_schema.mx 18 Apr 2007 09:16:26 -0000 1.123 @@ -602,11 +602,10 @@ { char *tname = qname_table(qname); sql_schema *ss = cur_schema(sql);
sql_trigger * t= NULL;
if (!schema_privs(sql->role_id, ss)) return sql_error(sql, 02, "DROP TRIGGER: access denied for %s to schema ;'%s'", stack_get_string(sql, "current_user"), ss->base.name);
if ((t = mvc_bind_trigger(sql, ss, tname )) == NULL)
- if (mvc_bind_trigger(sql, ss, tname ) == NULL)
Why I should not use the t? I found similar examples in the code. Maybe the the t was used after. I agree with this change I think it does affect the semantics of the function.
If you look at the code, the t was not used anywhere in that function. It was assigned a value, and that's it. That's what icc warns about (and rightly so!). Stefan's question boils down to the question, should that t have been used after it had been assigned, i.e. did you need to do anything with the result of the function call? If not, then the assignment and hence the variable was superfluous.
Sjoerd Mullender wrote:
Romulo Goncalves wrote:
Stefan Manegold wrote:
Update of /cvsroot/monetdb/sql/src/server In directory sc8-pr-cvs16:/tmp/cvs-serv14592/src/server
Modified Files: sql_schema.mx Log Message:
[since I spent (almost) all night and morning on compiling & testing, anyway:]
for the umpteenth time: make SQL compile with icc, again, by removind set but unused variable --- some more concentration during coding might save us quite some time ....
Romulo,
could you please check, whether the code still complies with you intention?
Index: sql_schema.mx
RCS file: /cvsroot/monetdb/sql/src/server/sql_schema.mx,v retrieving revision 1.122 retrieving revision 1.123 diff -u -d -r1.122 -r1.123 --- sql_schema.mx 17 Apr 2007 12:27:07 -0000 1.122 +++ sql_schema.mx 18 Apr 2007 09:16:26 -0000 1.123 @@ -602,11 +602,10 @@ { char *tname = qname_table(qname); sql_schema *ss = cur_schema(sql);
sql_trigger * t= NULL;
if (!schema_privs(sql->role_id, ss)) return sql_error(sql, 02, "DROP TRIGGER: access denied for %s to schema ;'%s'", stack_get_string(sql, "current_user"), ss->base.name);
if ((t = mvc_bind_trigger(sql, ss, tname )) == NULL)
- if (mvc_bind_trigger(sql, ss, tname ) == NULL)
Why I should not use the t? I found similar examples in the code. Maybe the the t was used after. I agree with this change I think it does affect the semantics of the function.
If you look at the code, the t was not used anywhere in that function. It was assigned a value, and that's it. That's what icc warns about (and rightly so!). Stefan's question boils down to the question, should that t have been used after it had been assigned, i.e. did you need to do anything with the result of the function call? If not, then the assignment and hence the variable was superfluous.
In the last checkin I removed the line where the t (the trigger) was used. Not using the icc compiler I did not realize that the t was not used anymore. You are right it was my mistake. Next time I will try to check this things.
Regards, Romulo
If you look at the code, the t was not used anywhere in that function. It was assigned a value, and that's it. That's what icc warns about (and rightly so!). Stefan's question boils down to the question, should that t have been used after it had been assigned, i.e. did you need to do anything with the result of the function call? If not, then the assignment and hence the variable was superfluous.
thanks for the "translation", Sjoerd! ;-)
In fact, this "picky" icc has detected a lot of real bugs in the code by complaining about unused variables, parameters and alike; that's why I asked Romulo to have a second look at the code --- no "complaint" from my side just a hint to accept and exploit the help that the compilers (and testing) provide us ...
Stefan
Index: sql_schema.mx
RCS file: /cvsroot/monetdb/sql/src/server/sql_schema.mx,v retrieving revision 1.122 retrieving revision 1.123 diff -u -d -r1.122 -r1.123 --- sql_schema.mx 17 Apr 2007 12:27:07 -0000 1.122 +++ sql_schema.mx 18 Apr 2007 09:16:26 -0000 1.123 @@ -602,11 +602,10 @@ { char *tname = qname_table(qname); sql_schema *ss = cur_schema(sql);
sql_trigger * t= NULL;
if (!schema_privs(sql->role_id, ss)) return sql_error(sql, 02, "DROP TRIGGER: access denied for %s to schema ;'%s'", stack_get_string(sql, "current_user"), ss->base.name);
if ((t = mvc_bind_trigger(sql, ss, tname )) == NULL)
- if (mvc_bind_trigger(sql, ss, tname ) == NULL) return sql_error(sql, 02, "DROP TRIGGER: unknown trigger %s\n", tname); mvc_drop_trigger(sql, ss, tname); return stmt_none();
Why I should not use the t?
well, of course you can use t, but the code above does (no longer) use t, and icc correctly tell us that declaring and setting a variable without using it does not make much sense...
I found similar examples in the code. Maybe the the t was used after.
for sure, t must be used in these other cases; otherwise, icc would (correcly!) complain.
since your yesterday's changes (removal of "mvc_drop_dependencies(sql, t->base.id);") t was not used any more; cf. http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?view... http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?r1=1...
in fact, you originally intorduced "mvc_drop_dependencies(sql, t->base.id);", and hence the need for t on Tue Oct 31 2006, cf., http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?view... http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?r1=1...
but then apparently forgot to clean-up properly, when removing "mvc_drop_dependencies(sql, t->base.id);", again ...
I agree with this change I think it does affect the semantics of the function.
you do agree although it does affect the semantics?
well, I hoped it would not change the semantics...
can you explain in what way it does change the semantics?
Stefan
Regards, Romulo
Stefan Manegold wrote:
Index: sql_schema.mx
RCS file: /cvsroot/monetdb/sql/src/server/sql_schema.mx,v retrieving revision 1.122 retrieving revision 1.123 diff -u -d -r1.122 -r1.123 --- sql_schema.mx 17 Apr 2007 12:27:07 -0000 1.122 +++ sql_schema.mx 18 Apr 2007 09:16:26 -0000 1.123 @@ -602,11 +602,10 @@ { char *tname = qname_table(qname); sql_schema *ss = cur_schema(sql);
sql_trigger * t= NULL;
if (!schema_privs(sql->role_id, ss)) return sql_error(sql, 02, "DROP TRIGGER: access denied for %s to schema ;'%s'", stack_get_string(sql, "current_user"), ss->base.name);
if ((t = mvc_bind_trigger(sql, ss, tname )) == NULL)
- if (mvc_bind_trigger(sql, ss, tname ) == NULL) return sql_error(sql, 02, "DROP TRIGGER: unknown trigger %s\n", tname); mvc_drop_trigger(sql, ss, tname); return stmt_none();
Why I should not use the t?
well, of course you can use t, but the code above does (no longer) use t, and icc correctly tell us that declaring and setting a variable without using it does not make much sense...
I found similar examples in the code. Maybe the the t was used after.
for sure, t must be used in these other cases; otherwise, icc would (correcly!) complain.
since your yesterday's changes (removal of "mvc_drop_dependencies(sql, t->base.id);") t was not used any more; cf. http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?view... http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?r1=1...
in fact, you originally intorduced "mvc_drop_dependencies(sql, t->base.id);", and hence the need for t on Tue Oct 31 2006, cf., http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?view... http://monetdb.cvs.sourceforge.net/monetdb/sql/src/server/sql_schema.mx?r1=1...
but then apparently forgot to clean-up properly, when removing "mvc_drop_dependencies(sql, t->base.id);", again ...
I agree with this change I think it does affect the semantics of the function.
Yes yes, I know that I just all my updates in this file. It was my fault. Sorry again. :)
It does not affect the semantics. Sorry I forgot the *not*
Regards, Romulo
you do agree although it does affect the semantics?
well, I hoped it would not change the semantics...
can you explain in what way it does change the semantics?
Stefan
Regards, Romulo
participants (3)
-
Romulo Goncalves -
Sjoerd Mullender -
Stefan Manegold