Hi, Kristofer!
On Oct 16, Kristofer Pettersson wrote:
> Sergei Golubchik skrev:
>>> === modified file 'sql/mysql_priv.h'
>>> --- a/sql/mysql_priv.h 2009-09-17 09:20:11 +0000
>>> +++ b/sql/mysql_priv.h 2009-10-12 13:34:13 +0000
>>> @@ -1120,9 +1120,11 @@ bool reload_acl_and_cache(THD *thd, ulon
>>> bool *write_to_binlog);
>>> #ifndef NO_EMBEDDED_ACCESS_CHECKS
>>> bool check_access(THD *thd, ulong access, const char *db, ulong
>>> *save_priv,
>>> - bool no_grant, bool no_errors, bool schema_db);
>>> -bool check_table_access(THD *thd, ulong want_access, TABLE_LIST *tables,
>>> - uint number, bool no_errors);
>>> + bool no_grant, bool no_errors, bool schema_db);
>>> +bool check_table_access(THD *thd, ulong requirements,TABLE_LIST *tables,
>>> + bool no_errors,
>>> + bool any_combination_of_privileges_will_do,
>>> + uint number);
>>> #else
>>
>> Unless you have a good argument to do it differently, please try to
>> keep the names are the order or similar arguments in similar
>> functions.
>>
>> Now check_table_access has (thd, requirements, tables, no_errors,
>> any_combination_of_privileges_will_do, number) while check_grant uses
>> (thd, want_access, tables, any_combination_will_do, number,
>> no_errors)
>
> I agree that it would be better to have a consistent order. The reason
> for the current order is to make it easier to track the back porting
> effort. If you don't see any increased risk I can perform this change
> right away.
first - I don't understand how that will make it easier to track the
back porting effort.
yes, I don't see any increased risk, but may be only because I don;t
understand the above.
> In a follow up patch I proposed for Kostja to remove the 'number' attribute
> from both signatures as it is construction which is prone to errors.
> However I considered it as a separate task.
yes, I agree it's a separate task.
and I don't understand why 'number' attribute is prone to errors.
> Do you agree that is a good idea? It would obviously mean that we look
> at those special cases where the TABLE_LIST contains more items then
> it should.
To take care of the special cases you'll probably need to unlink the
element from the TABLE_LIST before checking the privileges. And perhaps
add it back, although the latter may be unnecessary.
Regards / Mit vielen GrЭъen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB MЭnchen 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
GeschДftsfЭhrer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin HДring
