[KDE/Mac] Aligning the sockets in kdeinit5 and friends on OS X

[Ian Wadham]

Hello David,

And thank you very much for your interst and prompt reply.

On 02/02/2015, at 6:35 PM, David Faure wrote:
> On Monday 02 February 2015 15:24:21 Ian Wadham wrote:
>>    frameworks/kcrash/src/kcrash.cpp
>>    frameworks/kinit/src/kdeinit/kinit.cpp
>>    frameworks/kinit/src/wrapper.cpp
>> 
>> @David: Please can you confirm that these are the *only* relevant hits
>> and that they will catch all the cases where the socket name is generated.
> 
> My grep-fu is as good as yours ;)

I was worried that there might be cases like (appName + "_" + getDisplay()),
but if there are, I guess they will turn up sooner or later... ;-)

> I was wondering why klauncher was not in the list, after all it's the one 
> writing to that socket, to tell kinit what to do. The answer: because 
> klauncher is started by kdeinit forking and passing the file descriptor as 
> "command-line" argument (argv). So that explains it ;)

Watch this space… There was a glitch with that "argv" in KDE4 + OSX.  I will
check if it is still there in Frameworks + OSX.

>> Attached are three tentative patches to fix things so that the same name
>> is generated everywhere in Frameworks on Apple OS X.  I am proposing
>> to use "NODISPLAY", rather than the value of $DISPLAY or $MAC_DISPLAY.
>> One reason is to make sure wrapper.cpp and kinit.cpp use the same name.
> 
> Works. But looks weird, no?
> A Mac user looking at the socket name would really wonder what that NODISPLAY 
> means. I wonder why not just remove that…

I agree.  I did not like NODISPLAY either, but was "following suit" with Q_OS_WIN
in kcrash.cpp.  I do not like a bald, unexplained  "kdeinit5_" as a file name for the
socket.  How about a self-descriptive "kdeiit5_socket" (i.e. suffix = "socket")?

If you or René have no other suggestion I will go with "socket".

>> @David: Some last questions.  Why does kinit.cpp use an indirect method
>> called displayEnvVarName() to look for the socket name's suffix, but both
>> crash.cpp and wrapper.cpp use a direct method called getDisplay()?  
> 
> Waldo's brain is hard to dissect, so many years after the fact :-)

Waldo Bastian was one of the greats of KDE.  But I wish he had written some
comments in the code… :-)  Do you mind if I add some occasionally?  Subject
to review of whether my understanding is correct, of course.

> One data point: it looks like wrapper.cpp used to be plain C.

Yes, it did.  Some git bisecting may tell us more about why things are as they
are in kdeinit5 and friends, unless they were in Waldo's original code…

>> Should not all three be using identical code?  And could that be implemented
>> by using a macro or an include file, rather than cloning code?
> 
> That would be good. The reason it's not done currently is that the common 
> dependency for this code is the kinit framework, and that framework doesn't 
> ship a library, only executables. So this can't be a proper shared lib, but 
> indeed, you could try shipping the code all-inline or in a macro. A bit ugly, 
> but duplication is arguably ugly too (it's ugly, but more hidden....).
> I'm ok with kinit installing this code in a header file. A shared lib would be 
> overkill.

I agree with using a header file, but will not write one yet.  For one thing, the
Windows versions of this code are not aligned… :-)  Maybe there is a good
(Microsoftian) reason why.

When we are OK with the "tentative" patches, I will polish them to add some
explanation and change NODISPLAY.  Then I can put them on BKO and
Review Board, as Jeremy has suggested.

Cheers, Ian W.