[PATCH] Fix the last of the defaults and include a test to check them

Michael Adam obnox at samba.org
Sun Feb 2 06:10:35 MST 2014


On 2014-02-02 at 09:12 +1300, Andrew Bartlett wrote:
> On Sat, 2014-02-01 at 13:39 +0100, Michael Adam wrote:
> > On 2014-01-31 at 21:09 +0100, Michael Adam wrote:
> > > On 2014-02-01 at 09:05 +1300, Andrew Bartlett wrote:
> > > > 
> > > > I've just re-pushed all the outstanding branches. 
> > > 
> > > Alright will try to look after that, hopefully over the week-end.
> > 
> > I have looked over all the metadata patches.
> > I have tagged most of them as reviewed-by-me.
> > 
> > See my master-param branch:
> > https://git.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=master-param
> > 
> > But there are some questions:
> > (commits messages marked with "TODO(...) " and not flagged as
> > reviewed yet.)
> > 
> > For some of the parameters, when adding metadata, you don't add
> > a function or generated_function field. Is that missing?
> > Or what are the systematics here?
> 
> The idea is that, by default the generate-param.py script shouldn't need
> this.  Ideally the function should be the name of the parameter, with
> spaces becoming _.  Ideally the build system would always generate the
> matching function.  Sadly we are very inconsistent in our names here,
> and so much more metadata is required than I ever imagined, and what
> existed was so often incorrect.  Thankfully Garming was able to script
> the additions, but what looks like a special case was really meant to be
> the default/normal case :-).

OK understood - thanks.
But why don't we rename the functions to be consistent with
the parameter names (where possible)? That would seem the
logical step to me. This would also render most of the
30-something patches in which you add "function" metadata
unneccessary.

E.g. "add share command":

- in 833e8cb7a8f64f9cb2a30802954484bd125c5626 you changed the
  variable used for this from szAddShareCommand to add_share_cmd
  to match the function name.

- Wouldn't it be better to first change the function name
  to match the parameter name, i.e.  add_share_command"and then
  adapt the variable to match the function and hence the parameter
  name, thereby removing the special case?

- I guess that you didn't want to have to touch the callers
  thereby minimizing the scope of the patches, but I think
  this would be really justified.

==> I will present a few patches later.

One other question about metadata:
Could you provide a list of metadata entries that you
added and their precise meaning?
So far I saw "function", "synonym", "generated_function",
"constant", "parm"

> We also don't need it on synonyms (no function is generated in this
> case). 
> 
> > One other thing is that you changed the mask/mode parameters
> > to type integer. In the param-table, they are "octal", so I
> > was wondering whether there is anything special needed here?
> 
> You are entirely correct, this should be a different type.  We get away
> with it because for the things so far generated, they are 'just'
> integers.  
> 
> This and many other of the details encoded in the param_table.c (such as
> the ordering, sections, flags, special handler and enumeration types),
> will need to be added before we can generate that table. 

Ok.

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140202/6485138a/attachment.pgp>


More information about the samba-technical mailing list