On Jan 11, 2008 11:18 AM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Jan 11, 2008 12:58 AM, Dan McGee <dan@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@gmail.com> Signed-off-by: Dan McGee <dan@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