From owner-cvs-src@FreeBSD.ORG  Sat Jan  5 01:04:24 2008
Return-Path: <owner-cvs-src@FreeBSD.ORG>
Delivered-To: cvs-src@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 4470916A420;
	Sat,  5 Jan 2008 01:04:24 +0000 (UTC)
	(envelope-from brde@optusnet.com.au)
Received: from mail05.syd.optusnet.com.au (mail05.syd.optusnet.com.au
	[211.29.132.186])
	by mx1.freebsd.org (Postfix) with ESMTP id B148113C448;
	Sat,  5 Jan 2008 01:04:23 +0000 (UTC)
	(envelope-from brde@optusnet.com.au)
Received: from c211-30-219-213.carlnfd3.nsw.optusnet.com.au
	(c211-30-219-213.carlnfd3.nsw.optusnet.com.au [211.30.219.213])
	by mail05.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id
	m0514Hsk023281
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO);
	Sat, 5 Jan 2008 12:04:21 +1100
Date: Sat, 5 Jan 2008 12:04:17 +1100 (EST)
From: Bruce Evans <brde@optusnet.com.au>
X-X-Sender: bde@delplex.bde.org
To: Tim Kientzle <kientzle@freebsd.org>
In-Reply-To: <477E71F1.4080301@freebsd.org>
Message-ID: <20080105115225.T23063@delplex.bde.org>
References: <200801031830.m03IUb9K049549@repoman.freebsd.org>
	<477E71F1.4080301@freebsd.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
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
X-BeenThere: cvs-src@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: CVS commit messages for the src tree <cvs-src.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-src>,
	<mailto:cvs-src-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/cvs-src>
List-Post: <mailto:cvs-src@freebsd.org>
List-Help: <mailto:cvs-src-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-src>,
	<mailto:cvs-src-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 05 Jan 2008 01:04:24 -0000

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