Re: [PATCH 2/6] move copyfileperms to non-static file_copyfileperms
Hi!
Same for this one, comments on the minor modifications follow.
On Mon, 2009-09-28 at 23:34:29 +0200, Sean Finney wrote:
> this functionality is also needed by the conffile handling code to ensure
> that the merge output is stored in a file with the same permissions as
> the original conffile, preventing the accidental oppurtunity for
> unintended information disclosure.
>
> therefore the function is moved into a new library module (file.{c,h}),
> and given an appropriate prefix. note that some of the translatable error
> messages have been modified as they would otherwise be misleading.
Capitalizaion. One space after dot.
> ---
> lib/dpkg/Makefile.am | 1 +
> lib/dpkg/file.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/dpkg/file.h | 35 +++++++++++++++++++++++++++++
> src/configure.c | 28 +----------------------
> 4 files changed, 97 insertions(+), 26 deletions(-)
> create mode 100644 lib/dpkg/file.c
> create mode 100644 lib/dpkg/file.h
> diff --git a/lib/dpkg/file.c b/lib/dpkg/file.c
> new file mode 100644
> index 0000000..0408844
> --- /dev/null
> +++ b/lib/dpkg/file.c
> @@ -0,0 +1,59 @@
[...]
> +#include <config.h>
> +#include <compat.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +#include <dpkg/file.h>
> +#include <dpkg/dpkg.h>
> +#include <dpkg/i18n.h>
Sorted a bit the includes.
> +/*
> + * Copy file ownership and permissions from one file to another.
> + */
Moved this comment to the header file. At some point we should start
properly documenting the functions and variables (at least for the
stuff exposed by libdpkg), with something like doxygen. I've an old
Doxyfile around for dpkg that might need to tunning before committing.
> +void
> +file_copyfileperm(const char *source, const char *target)
> +{
> + struct stat stab;
> +
> + if (stat(source, &stab) == -1) {
> + if (errno == ENOENT)
> + return;
> + ohshite(_("unable to stat installed file `%.250s'"), source);
> + }
> +
> + if (chown(target, stab.st_uid, stab.st_gid) == -1)
> + ohshite(_("unable to change ownership of target file`%.250s'"),
> + target);
> +
> + if (chmod(target, (stab.st_mode & 07777)) == -1)
> + ohshite(_("unable to set mode of target file`%.250s'"), target);
> +}
Rename this to file_copyfileperm file_copy_perms, the former seems too
redundant. Also fixed the missing spaces, the old quotes and the strings a
bit.
> +/*
> + * vim: noet ts=8
> + */
Removed this one.
thanks,
guillem
Reply to: