Remaining "cast"/"const" patches

[Andrew Bartlett]

On Sun, 2009-10-18 at 16:25 +0000, Matthias Dieter Wallnöfer wrote:
> Andrew,
> 
> I finished the split-up. It's not reasonable to post the patches as attachment - so they're only in my private repo. Maybe you like to write the filenames or patch titles of those which look okay.

I've tried to comment on these a bit more, so you can have a chance to
fix them.  In short, all the changes need work and review, but I hope
that these thoughts can clarify the situation a bit more, as regards
these particular patches.

As such, please do not merge any of the const patches in their current
form. 

A lot of these patches would be much more acceptable if a function
were developed that took a char ** and returned a const char **.  This
would avoid a *lot* of casts, and be able to enforce the type safety
of the operation.

    s4:torture/rpc/wkssvc - Add better cast
Perhaps instead change the type of *msg and put the cast on the
assignment into r.in.message_buffer?  But I think the IDL is wrong
here - we should very rarely do manual character set conversion in
Samba.  We should check with dochelp first about this, as it's not in
the WSPP docs. 

    s4:torture/raw/samba3misc - Add "discard_const_p" macro before a
string
I've merged this patch. 

    s4:torture/nbt/wins - Add more casts
This needs the casting function I talked about above.

    s4:torture/nbench/nbench - Add a cast before "str_list_make_shell"
This needs the casting function I talked about above.

    s4:torture/ldap/ldap_sort - Add some casts to suppress warnings
I agree this test is a bit of a mess, but I'm not really sure what to
do about it.  I don't think these casts are the right solution.

    s4:param/loadparm - Add new casts (needed due to API changes of
"str_list_equal")
I'm not sure about this (the casts are really hairy either way)

    s4:ntvfs/posix - Remove some "const"
This commit message is highly misleading, as you add a talloc_strdup().
Why do you need that?

(An extra talloc seems harmless - but I managed to slow down Samba by
2x with a single extra talloc in my partition changes!)

    s4:nbt_server/wins/winsdb - Remove "const" in front of "str"
This would be better as data_blob_string_const()

    s4:ldb_map - Remove "const" from search tree
This needs a more complete fix.  ldb_map modifies the search tree, and
it should not. 

    s4:schema - Make some more (result) variables const and fix up
warnings with "discard_const_p"
This I almost like, except the way we lie and claim the 'schema' is
const.  We should instead have a way to get a 'non-const' schema
(perfectly valid, as we are still constructing it at this point).

With a bit more work, we may be able to merge this. 

    s4:dsdb/samdb/ldb_modules/acl - Add "const" and casts and remove
unused variables
Please either forward to nivanova or skip for now, to avoid merge
problems. 

    nsswitch/pam_winbind: "CONST_DISCARD" isn't needed at all here
Perhaps make investigations as to why this was added in the first
place?  Preusmably older iniparser libraries are not const safe.

    libcli/nbt/nbtname: Attempt made to remove "discard_const_p" call
I don't see how the change in ndr_pull_nbt_string() removes a
discard_const_p(), and it changes behaviour.  It may be a correct
change, but it needs to be seperate and justified.

I see it it partly to then make ndr_pull_nbt_string() simpiler, but
have you looked for other callers, and can you better explain what you
are doing.  Either way, this and any other change outside source4/
must be specifically checked with Volker (as he already expressed an
objection).

    libcli/named_pipe_auth/npa_tstream: Remove a "const" from prototype
of "tstream_npa_connect_send" to prevent "discard_const_p"
Check this with metze, but did you try changing 'struct
named_pipe_auth_req_info3 *info3' instead?  Doesn't this just push the
problem to the caller?

    util/strlist: Fix up "const" warnings in the string list and test
code
Why do we need this when casting from a void *?
	const struct test_list_element *element =
				(const struct test_list_element *) data;

This also needs a 'char ** to const char **' function I think.

    charset/tests/iconv - Remove the "const" from "ptr_in" to prevent
"discard_const_p" use
    tdb tools: Cosmetic adaptions and substitute one "free" with
"SAFE_FREE"
These should be put past Rusty (defacto TDB maintainer) for comment,
but why is SAFE_FREE better here?  This is a stack variable about to
go out of scope.

    mount.cifs: Fix "discard const pointer" warning through introducing
the "discard_const_p" macro
These should be put past those maintaining mount.cifs (Steve French et
al)

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091028/3a8a4b28/attachment.pgp>