httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: cvs commit: apache-2.0/src/lib/apr/file_io/win32 filestat.c
Date Mon, 03 Jul 2000 21:38:50 GMT
On Mon, Jul 03, 2000 at 08:43:04AM -0700, rbb@covalent.net wrote:
> On 3 Jul 2000 gstein@locus.apache.org wrote:
> > gstein      00/07/03 05:06:40
> > 
> >   Modified:    src/lib/apr/include apr_file_io.h
> >                src/lib/apr/file_io/os2 filestat.c
> >                src/lib/apr/file_io/unix dir.c fileacc.c fileio.h filestat.c
> >                         open.c pipe.c
> >                src/lib/apr/file_io/win32 filestat.c
> >   Log:
> >   add ap_finfo_t.device
> >   add ap_setfileperms() for setting file permissions (chmod cover).
> >       - OS/2 and Win32 currently return APR_ENOTIMPL
> >   fix the file perm handling in APR: some conversion between ap_fileperms_t
> >       and mode_t was not occurring; adding new conversion function; renamed
> >       old conversion func.
> >   
> 
> Why were these done in one commit?  These are three VERY distinct
> patches.

It was small and easy work, easy to review, and it was all the same set of
files. Also, I've seen much larger and disparate changes checked in as a
batch before; this didn't even seem close to those.

> I would also like to understand why the function was
> renamed.  This looks like a personal preference as far as the name goes,
> which IMHO is not a great reason for the change.  It isn't worth it to
> rename it back, but this sure looks like a change for the sake of change.

I introduced a second function with the opposite semantics. The rename was
done to create a matched pair of function (ap_unix_perms2mode and
ap_unix_mode2perms). The name also made it clearer what the function did
(ap_unix_get_fileperms() returned a mode_t given an ap_fileperms_t; the new
name makes this a bit more obvious)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message