When I enabled error reporting, I noticed Category.inc was throwing up a LOT of errors like this:

Notice: Undefined property: stdClass::$79 in /var/www/html/dbptest/modules/category/category.inc on line 1444

Looking at the code, it looks like this...

        while ($item = db_fetch_object($result)) {
          $cache[$type][$item->$id] = $item;
          $i++;
        }

Specifically the $cache[$type] line. Why is it referring to $item->$id (bearing in mind that $id seems to be the current node you're viewing). Should this line not be:
$cache[$type][$item->nid] = $item;

I change it to that on mine and the errors disappeared - whats the impact?

PS: I have made no patch - its a 4 byte change. Its not hard to do yourself :-)

CommentFileSizeAuthor
#8 category.cache.patch3.57 KBbdragon

Comments

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

Can anyone confirm this or at least tell me if I'm talking crap! :-) Cheers.

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

Ok - I think I made a mistake, it should be cid not nid... I think!

bdragon’s picture

bdragon commented
Status: Needs review » Active

Agreed, the code is bogus.

bdragon’s picture

bdragon commented

Also, I think $cache[$type][$id] = $item; would be the intent.

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

I doubt it because $type and $id dont change in that loop. You'd end up setting the same variable to loads of different things repeatedly.

Maybe that whole function needs better remarks in it - I find some of it quite confusing, like what is $overflow_from for?

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

Ok - I just did a clean install on a site and I'm coming up with wierd errors. Sometimes $item contains a number of properties (cid, cnid, description, weight, depth & title) and sometimes it contains only a nid.

It doesn't help that this function has next to no comments in it so i have no idea what that array is expecting. Does it need a cid or a nid?

bdragon’s picture

bdragon commented

The code is broken. Period. Commenting it out doesn't hurt anything.

bdragon’s picture

bdragon commented
Status: Active » Needs work
StatusFileSize
new category.cache.patch3.57 KB

This was sitting in my patch directory for a couple of weeks.
This patch was my attempt at optimizing the function. It's kind of ugly, but I think it works. (Havent touched it since Oct 4)

bdragon’s picture

bdragon commented
Title: PHP Notice Error » category_get_cached_item broken

Changing title.

JirkaRybka’s picture

JirkaRybka commented
Status: Needs work » Closed (won't fix)

This code survived the 5.x era (somehow - I don't really know whether this got really fixed and how), and was finally removed, early in the 5.x->6.x porting process. Later, it was replaced by a completely different approach at #501378: PERFORMANCE! Central caching for category API functions.

So, this issue is definitely obsolete, as it's about a bit of code that no longer exists in Category package. (4.7.x and 5.x versions are not supported now.)