Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 18 Jun 2010 18:23:46 -0700
From:      Garrett Cooper <gcooper@FreeBSD.org>
To:        Julien Laffaye <jlaffaye@freebsd.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 179817 for review
Message-ID:  <AANLkTinFOtQBdMMVRn88g69mHao3tQx99nF6zbKhhdai@mail.gmail.com>
In-Reply-To: <201006190031.o5J0VT62010908@repoman.freebsd.org>
References:  <201006190031.o5J0VT62010908@repoman.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jun 18, 2010 at 5:31 PM, Julien Laffaye <jlaffaye@freebsd.org> wrot=
e:
> http://p4web.freebsd.org/@@179817?ac=3D10
>
> Change 179817 by jlaffaye@jlaffaye-chulak on 2010/06/19 00:30:38
>
> =A0 =A0 =A0 =A0Errors checking, comments, ...
>
> Affected files ...
>
> .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/complete/ma=
in.c#3 edit
> .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/complete/pe=
rform.c#4 edit
>
> Differences ...
>
> =3D=3D=3D=3D //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/c=
omplete/main.c#3 (text+ko) =3D=3D=3D=3D
>
> @@ -28,6 +28,10 @@
> =A0 =A0 =A0 =A0if ((dir =3D dirname(argv[1])) =3D=3D NULL)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err(1, "dirname(%s)", argv[1]);
>
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* Take the real path of the target file
> + =A0 =A0 =A0 =A0* because we are going to chdir()
> + =A0 =A0 =A0 =A0*/
> =A0 =A0 =A0 =A0if ((realpath(argv[2], out)) =3D=3D NULL)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err(1, "realpath(%s)", argv[2]);
>
> @@ -40,6 +44,6 @@
> =A0static void
> =A0usage(void)
> =A0{
> - =A0 =A0fprintf(stderr, "usage: pkg_complete source_package target_packa=
ge\n");
> - =A0 =A0exit(1);
> + =A0 =A0 =A0 fprintf(stderr, "usage: pkg_complete source-package target-=
package\n");
> + =A0 =A0 =A0 exit(1);
> =A0}

It might be a good point to ask, what options are and aren't supported
that can be passed to package_complete (from a user perspective)?

This should have been hashed out earlier, but it never hurts to ask now..

> =3D=3D=3D=3D //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/c=
omplete/perform.c#4 (text+ko) =3D=3D=3D=3D
>
> @@ -22,23 +22,31 @@
> =A0 =A0 =A0 =A0struct stat st;
> =A0 =A0 =A0 =A0struct list_deps deps;
> =A0 =A0 =A0 =A0char fname[PATH_MAX], buf[1024], *ext;
> - =A0 =A0 =A0 size_t r;
> + =A0 =A0 =A0 ssize_t r;
> =A0 =A0 =A0 =A0int fd, err =3D 0;
>
> - =A0 =A0 =A0 if ((ext =3D strrchr(pkgname, '.')) =3D=3D NULL)
> + =A0 =A0 =A0 if ((ext =3D strrchr(pkgname, '.')) =3D=3D NULL) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warn("strrchr()");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (1);
> + =A0 =A0 =A0 }
>
> =A0 =A0 =A0 =A0deps.size =3D 100;
> =A0 =A0 =A0 =A0deps.len =3D 0;
> =A0 =A0 =A0 =A0deps.pkgs =3D malloc(sizeof(Package)*(deps.size));

What if this fails?

> + =A0 =A0 =A0 =A0 =A0 =A0=3D=3D -1) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 warnx("error while unpacking %s", CONTENTS_=
FNAME);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (1);
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 pkg.head =3D pkg.tail =3D NULL;
> + =A0 =A0 =A0 read_plist_from_buffer(&pkg, plist_buf, plist_size);
> + =A0 =A0 =A0 free(plist_buf);

What if this fails?

> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Register the current package */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (deps->size <=3D deps->len) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 deps->size *=3D 2;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 deps->pkgs =3D realloc(deps=
->pkgs,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0sizeof(Package)*(deps->size));
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 deps->pkgs[deps->len] =3D pkg;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 deps->len++;
...

> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 * Get the dependencies of pkgname's dependencies,
> + =A0 =A0 =A0 * if not already done.
> + =A0 =A0 =A0 */
> + =A0 =A0 =A0 for (p =3D pkg.head; p; p =3D p->next) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (p->type =3D=3D PLIST_PKGDEP) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 found =3D 0;

It'd be nice if found this was TRUE/FALSE.

Thanks!
-Garrett



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinFOtQBdMMVRn88g69mHao3tQx99nF6zbKhhdai>