Re: GWorkspace Segfault

[Riccardo Mottola]

Hi Fred,

On 11/24/13 12:41, Fred Kiefer wrote:
> Oops, I wasn't aware of that. Yes of course, there is more to remove.
> All that similar code in FSNTextCell that is never used. And in
> FSNBrowserCell the fontAttr variable is a static although it is used by
> different cells which might have different fonts. Obviously this isn't
> going to work. I think it should just be a local variable in the
> cuttitle method.
Yes it is dubious. It is however dynamically set in setFont, at least 
that. Perhaps it is supposed that all cells will have the same font?

Whiel trying to make infoFont and fontAttr non-static, I got puzzled and 
made GWorkspace crash again, so let's move gradually.
The first thing: is infoFont actually needed? It is used aparently only 
for the dots, but the whole cell should use its own font (also since it 
implements setFont and re-sets fontAttr in there).

I thus removed infoFont from everywhere and substituted it with just 
[self font] where needed, using NSCell's. This even saves memory :)
What do you think? It works, I just commited and you sure have seen the 
patch flowing in.
> I also don't quite see why the cuttitle method gets cached. This just
> obscures the code and doesn't speed up much.
Well, the method is called for each cell in a browser thus it was 
thought to cache it.

What I do wonder is the very strange way to cache it, I have never seen 
that and I don't understand it.

Look at this:
typedef NSString *(*cutIMP)(id, SEL, id, float);

that's the only typedef for cutIMP and I can hardly get what it means, 
the IMP variable is then declared this way:
static cutIMP cutTitle = NULL;

and used this way:
   cutTitle = (cutIMP)[self instanceMethodForSelector: cutTitleSel];

and invoked this way:
cuttitle = (*cutTitle)(self, cutTitleSel, uncuttedTitle, textlenght);


I tried to rewrite it the way I have seen done in other places, by just 
using "IMP". Please look at the attached patch.
The main difference is

static IMP cutTitleImp = NULL;

(rewriting is just a first step to make it then outside initialize, but 
I am tracking down the error and ask for help).

The attached patch breaks. The method gets called (!) but by an NSLog 
inside, the last float parameter textlenght is always passed as 0.

Why ????


Riccardo

> This method also doesn't take into account the paragraph style (writing
> direction and alignment). You would need something like NSCells
> _nonAutoreleasedTypingAttributes method for this, but this isn't public
> and doesn't exist on Cocoa.
Are you referring to having file-names written right-to-left for example?
Well, that's perhaps a task for the future. First let's remove crashes 
and optimize stuff.
> Don't get me started on code reviewing these classes. You could get back
> a lot more input than you could handle :-)
Not all a the same time. But there are several classes in FSNode that I 
want to clean up and don't really know how :) Thus slowly... it will improve



Riccardo

imp.patch
Description: Text document