Date: Sat, 5 Jan 2008 12:04:17 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Tim Kientzle <kientzle@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, Dag-Erling Smorgrav <des@freebsd.org> Subject: Re: cvs commit: src/lib/libarchive archive_endian.h archive_read_support_format_zip.c Message-ID: <20080105115225.T23063@delplex.bde.org> In-Reply-To: <477E71F1.4080301@freebsd.org> References: <200801031830.m03IUb9K049549@repoman.freebsd.org> <477E71F1.4080301@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 4 Jan 2008, Tim Kientzle wrote: > Dag-Erling Smorgrav wrote: >> ... >> 1.1 +142 -0 src/lib/libarchive/archive_endian.h (new) >> 1.19 +22 -57 src/lib/libarchive/archive_read_support_format_zip.c > > Is this really right? > >> +be32dec(const void *pp) >> +{ >> + unsigned char const *p = (unsigned char const *)pp; > > The "const *" is harmless enough, but dropping the > leading "const" doesn't seem right at all: 'pp' > is a pointer to const data, 'p' points to > modifiable data. Um, both point to const data. `unsigned char * const p' would point to non-const data, and casting to that should cause a diagnostic. This is just a style bug -- the const qualifier is placed in the usual but confusing place for pp and in the unusual but easier to understand place for p. Other bugs in the above include namespace pollution (at least for the original copy in <sys/endian.h>) -- both p and pp are in the application namespace. This bug is missing in older MD endian.h code. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080105115225.T23063>