[pacman-dev] [PATCH] Use dynamic string allocation in package structures

[Dan McGee]

On Jan 11, 2008 11:18 AM, Aaron Griffin <aaronmgriffin at gmail.com> wrote:
> On Jan 11, 2008 12:58 AM, Dan McGee <dan at archlinux.org> wrote:
> > This also affects all structures with static strings, such as depmiss,
> > conflict, etc. This should help a lot with memory usage, and hopefully make
> > things a bit more "idiot proof".
> >
> > Currently our pactest pass/fail rate is identical before and after this
> > patch. This is not to say it is a perfect patch- I have yet to pull valgrind
> > out. However, this should be quite safe to use in all situations from here
> > on out, and we can start plugging the memleaks.
> >
> > Original-work-by: Aaron Griffin <aaronmgriffin at gmail.com>
> > Signed-off-by: Dan McGee <dan at archlinux.org>
> > ---
> > It works! All of the pactests currently passing still pass, and those that
> > previously failed also still fail.
> >
> > Let me know if anything seems weird in this patch.
>
> Wow. What was the kicker here? I know I had like 80% of pactests
> failing, presumably to segfaults, last I looked. You said you got it
> to like 66%, and now a day later it's back to normal. There had to be
> one big impedance here.

@@ -454,33 +460,39 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info,
pmdbinfrq_t inforeq)
                while(!feof(fp)) {
                        fgets(line, 255, fp);
                        _alpm_strtrim(line);
-                       if(!strcmp(line, "%DEPENDS%")) {
+                       if(strcmp(line, "%DEPENDS%") == 0) {
                                while(fgets(line, 512, fp) &&
strlen(_alpm_strtrim(line))) {
-                                       pmdepend_t *dep = alpm_splitdep(line);
+                                       pmdepend_t *dep =
alpm_splitdep(_alpm_strtrim(line));
                                        info->depends =
alpm_list_add(info->depends, dep);
                                }
-                       } else if(!strcmp(line, "%OPTDEPENDS%")) {
+                       } else if(strcmp(line, "%OPTDEPENDS%") == 0) {
                                while(fgets(line, 512, fp) &&
strlen(_alpm_strtrim(line))) {
-                                       info->optdepends =
alpm_list_add(info->optdepends, strdup(line));
+                                       char *linedup;
+                                       STRDUP(linedup,
_alpm_strtrim(line), goto error);
+                                       info->optdepends =
alpm_list_add(info->optdepends, linedup);
                                }


Here is the explanation for those curious (mostly Aaron but Xavier asked too).

Depends are now stored in our package struct as a list of pmdepend_t
objects, not strings. When Aaron originally started work on this
patch, it may have still been stored as strings, or due to an
overzealous replacement plan, it was substituted with the standard
string storage code. You can see the difference in the above two
sections from pkg_load. Originally the DEPENDS code was just like
OPTDEPENDS with the STRDUP call, but as you can see a splitdeps call
is needed instead, which handles any malloc-ing. This made everything
pass once I made the change.

-Dan