[PATCH] Fix up discard-const warnings in s4 and tries to remove "discard_const_p"s/"CONST DISCARD"s in both s3 and s4

Andrew Bartlett abartlet at samba.org
Tue Sep 29 19:24:17 MDT 2009


(I note that tridge has already given a very detailed and more thorough
reply, but as I already composed this, I'll send it anyway :-)

On Tue, 2009-09-29 at 14:56 +0200, Matthias Dieter Wallnöfer wrote:
> Hi metze,
> 
> thanks for your quick response.
> 
> Stefan (metze) Metzmacher schrieb:
> > I just read the first few patches, while some of the patches are really
> > nice to have, they're still very large bulk commits combining unrelated
> > changes.
> >   
> For each file a own commit - I don't know - then we had approximately 
> 150 of them.

This should give you an idea of the scale of the change you are
proposing.

This is a well trodden path, one that I started on similarly when I
joined the team.  We don't add these warnings just for fun - they have
been allowed to persist in the codebase because they are hard to fix
properly. 

In particular, our habit of passing around structures that contain a
pointer and a length is very hard to make properly const-safe.  Heimdal
does this in parts by having two structures, one for 'const' principals
and one for 'non const'.  Then you can have a function that casts
those. 

We have not decided to go down that road for DATA_BLOB and TDB_DATA,
which makes this difficult. 

(Also, your proposed fixes for Heimdal should be forwarded directly to
the heimdal-bugs alias, because I'm unwilling to diverage our branch of
Heimdal for compiler warnings.  See www.h5l.org.  We will eventually get
them when I resync). 

> > I think a some of the changes are not needed and some introduce real
> > bugs, e.g. the talloc_strdup() for the uint8_t buffers in the
> > gensec_gssapi and schannel_sign code.
> >   
> Where exactly lies the problem here? Memory? (So I can understand the 
> problem)

If the buffer isn't a NULL terminated string (say perhaps it's just
memory), then a talloc_strdup() could duplicate only part of the
structure. 

I commend your efforts in trying to reduce the warnings here, but I
still think there is a long way to go. 

As I said on IRC, please produce patches in 3 groups (and split them up
into per-file patches)
  - ones that only fix prototypes
  - ones that adds more const
  - ones that add talloc calls

Fixing this well is harder than it looks, so please be patient.  Indeed,
many of the changes may not be ever be merged. 

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/20090930/ac76d059/attachment.pgp>


More information about the samba-technical mailing list