On Tuesday, 02 May 2006 at 20:43, Rocco Rutte wrote: > Hi, > > * Brendan Cully [06-05-02 13:25:30 -0700] wrote: > > >I never got the point of FREE vs safe_free - I don't see it as a bug > >to use safe_free, though it is an inconsistency. > > If I recall correctly, some years ago there was: > > safe_free (void**); > #define FREE(X) safe_free((void**)(X)) > > just to avoid having to write all the casts manually. > > Now it's even more dangerous since safe_free() dereferences its > pointers without a big fat warning somewhere. In fact, before adding the > check_sec.sh checks I though about just adding a note somewhere but > couldn't really find an accurate place for it... That's why I don't like the automatic cast - the compiler should get a chance to check whether the argument is a ** type. That may be why the current macro is # define FREE(x) safe_free(x) But I don't think I understand why safe_free is void safe_free (void *) instead of void safe_free (void **) wouldn't this be the proper fix? > Now for "core" mutt this isn't interesting IMHO, just for people who > write patches. > > I've removed the PATCHES/check_sec.sh changes and saved it as 'p1' in > that directory which only replaces safe_free() by FREE(). > > Personally, I'd rather take to the time to double-check the use and keep > the check_sec.sh part since the first chunk in muttlib.c really looks > like a bug which could have been found that way. (and check_sec.sh would > only complain if there's no ampersand before the operator to FREE() so > not all FREE() calls are affected) It does look like a real bug, but wouldn't it be better to fix the prototype for safe_free?
Attachment:
pgpiObsWECHLI.pgp
Description: PGP signature