[pacman-dev] [PATCH 3/4] Change strcpy function using for strncpy

[Laszlo Papp]

On Sun, Oct 18, 2009 at 8:42 AM, Gerardo Exequiel Pozzi <
vmlinuz386 at yahoo.com.ar> wrote:

> Laszlo Papp wrote:
> >       * Size examined str* function usage is a common coding practice,
> because it's
> >       more safer to avoid breakage while using str* functions.
> >
> > Signed-off-by: Laszlo Papp <djszapi at archlinux.us>
> > ---
> >  src/pacman/pacman.c |    4 ++--
> >  src/pacman/query.c  |    4 ++--
> >  src/pacman/util.c   |    2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> > index 5e824f4..0aaf6f0 100644
> > --- a/src/pacman/pacman.c
> > +++ b/src/pacman/pacman.c
> > @@ -960,11 +960,11 @@ static void cl_to_log(int argc, char* argv[])
> >               return;
> >       char *p = cl_text;
> >       for(i = 0; i<argc-1; i++) {
> > -             strcpy(p, argv[i]);
> > +             strncpy(p, argv[i], strlen(argv[i]));
> >               p += strlen(argv[i]);
> >               *p++ = ' ';
> >       }
> > -     strcpy(p, argv[i]);
> > +     strncpy(p, argv[i], strlen(argv[i]));
> >       alpm_logaction("Running '%s'\n", cl_text);
> >       free(cl_text);
> >  }
> > diff --git a/src/pacman/query.c b/src/pacman/query.c
> > index 6b6a25d..cb15852 100644
> > --- a/src/pacman/query.c
> > +++ b/src/pacman/query.c
> > @@ -326,7 +326,7 @@ static int check(pmpkg_t *pkg)
> >               pm_fprintf(stderr, PM_LOG_ERROR, _("root path too
> long\n"));
> >               return(1);
> >       }
> > -     strcpy(f, root);
> > +     strncpy(f, root, rootlen);
> >
> >       const char *pkgname = alpm_pkg_get_name(pkg);
> >       for(i = alpm_pkg_get_files(pkg); i; i = alpm_list_next(i)) {
> > @@ -337,7 +337,7 @@ static int check(pmpkg_t *pkg)
> >                       pm_fprintf(stderr, PM_LOG_WARNING, _("file path too
> long\n"));
> >                       continue;
> >               }
> > -             strcpy(f + rootlen, path);
> > +             strncpy(f + rootlen, path, strlen(path));
> >               allfiles++;
> >               /* use lstat to prevent errors from symlinks */
> >               if(lstat(f, &st) != 0) {
> > diff --git a/src/pacman/util.c b/src/pacman/util.c
> > index f4e1756..4afed1c 100644
> > --- a/src/pacman/util.c
> > +++ b/src/pacman/util.c
> > @@ -352,7 +352,7 @@ char *strreplace(const char *str, const char *needle,
> const char *replace)
> >
> >       if(*p) {
> >               /* add the rest of 'p' */
> > -             strcpy(newp, p);
> > +             strncpy(newp, p, strlen(p));
> >               newp += strlen(p);
> >       }
> >       *newp = '\0';
> >
> Hi
>
> I am wrong or this change does not change nothing?
>
> All your changes are of this form:
>
> -strcpy(dst, src);
> +strncpy(dst, src, strlen(src));
>
> So if the lenght of source is greater than destination, both always will
> do a bad job. Maybe you intented to say:
>
> +strncpy(dst, src, strlen(dst));
>
> but this have another problem, so need at least another check, and or
> implement a strlcpy() from BSD
>
> Good Luck!
>
>
> --
> Gerardo Exequiel Pozzi ( djgera )
> http://www.djgera.com.ar
> KeyID: 0x1B8C330D
> Key fingerprint = 0CAA D5D4 CD85 4434 A219  76ED 39AB 221B 1B8C 330D
>
>
>
Hello djgera!

Yeah, you're right absolutely, I sent wrong patch, thanks the feedback.

I wanted to code it:

sdst = sizeof(dst)-1;
ssrc = sizeof(src)

n = (sdst < ssrc ? sdst : ssrc) ; strncpy (dst, src , n); dst[n] = '\0';

And if it will be used a lot, maybe inline function for it(alpm_strncpy).

Best Regards,
Laszlo Papp