From: PCMan <pcm...@gm...> - 2009-11-19 10:00:00
|
Yes, at least you can return NULL for other OSes to indicate that cwd is not available, so we can use fallback value g_get_current_dir(). For me, copying the Solaris code from gnome-terminal without testing is acceptable since this function is trivial. Code review should be enough. Otherwise, if any Solaris user encounters problems, we'll get bug report. Fred, as original author of lxterminal, do you have any comment on this? On Thu, Nov 19, 2009 at 1:31 PM, CHENG Renquan <rq...@sm...> wrote: > On Thu, Nov 19, 2009 at 1:13 PM, PCMan <pcm...@gm...> wrote: >> Thanks for your patch. >> I, however, found that the patch is not portable. >> Since getting cwd from pid is OS-specific stuff, I'd suggest >> encapsulate this in a function, and do some conditional compilation >> for different OSes inside. >> >> For example: >> >> char* get_cwd_for_pid(pid_t pid) >> { >> char* cwd = NULL; >> #ifdef __linux__ >> /* read cwd from /proc/... */ >> #elif defined(some_other_OS) >> /* some OS-specific code here */ >> #elif defined(yet_another_OS) >> /* some OS-specific code here */ >> #endif >> return cwd; >> } >> >> You can find how to do this in the source code of gnome-terminal, >> roxterm, or sakura (patched by me previously). > > Yes, I know it's not portable, I have read gnome-terminal code, it > supports both Linux and Solaris >= 10, in > gnome-terminal/src/terminal-screen.c; the problem now is I don't have > other operating systems, I can add this kind of code on, but cannnot > test it; and no interest with other os; > > So would you mind integrating this first, and then some time later if > someone else want to request for or add other os support, he will do > it; seems reasonable? > > char* > terminal_screen_get_current_dir (TerminalScreen *screen) > { > static const char patterns[][18] = { > "/proc/%d/cwd", /* Linux */ > "/proc/%d/path/cwd", /* Solaris >= 10 */ > }; > > -- > from Cheng > |