Re: [PATCH] mc crashes when temporary directory cannot be created

2006-11-30 Thread Jindrich Novy
Hi Leonard,

On Wed, 2006-11-29 at 20:59 +0100, Leonard den Ottolander wrote:
 Hello Jindrich,
 
 On Tue, 2006-11-28 at 13:21 +0100, Jindrich Novy wrote:
  IMO only removal of the fallback will prevent
  the infinite loop in any case as it shouldn't call mc_mkstemps() at all.
 
 That cure seems worse than the disease. Isn't the real problem the fact
 that mc_mkstemps() blindly concats tmpdir to the prefix instead of
 testing if mc_tmpdir() succeeded? Why not abort mc_mkstemps() if
 mc_tmpdir() returns /dev/null/?

I'm not quite sure I got the point. We may want to prevent infinite
loops caused by the subsequent function calls, such design is dangerous
by design. The fallback will never work without adding hacks to
mc_mktemps()/mc_tmpdir() to check for special arguments what doesn't
look too nice to me.

Considering that insufficient space in TMPDIR is very rare, I would vote
for removing the dangerous fallback completely what is done in the
previous patch. mc works fine after that even without the fallback.

 What about the attached (untested) patch?

$ TMPDIR=/dev/null mc
Cannot create temporary directory /dev/null/mc-jnovy: Not a directory
(20)
Cannot create temporary directory /dev/null/mc-jnovy: Not a directory
(20)

snip

Cannot create temporary directory /dev/null/mc-jnovy: Not a directory
(20)
Cannot create temporary directory /dev/null/mc-jnovy: Not a directory
(20)
Segmentation fault


 By the way, could you please add the -p option to your diffs?

Sure.

Jindrich

___
Mc-devel mailing list
http://mail.gnome.org/mailman/listinfo/mc-devel


Re: [PATCH] Help Viewer - incorrect behaviour of up arrrow key

2006-11-30 Thread Pavel Tsekov
Hello Grigory,

On Wed, 29 Nov 2006, Grigory Trenin wrote:

 Here is a detailed description how to reproduce it:

 1) Open the Help Contents (press F1, Tab, Enter)
 2) Navigate some lines forward (press End or PageDown several times)
 3) Enter the selected topic (press Enter)
 4) Return back (press right arrow)
 5) Move to the top of Contents (press Home)
 6) Now try navigating the Contents using the up and down arrow keys.
 (for example, press down arrow for 5 times, then press up arrow).
 You will notice that when you press up arrow key the selection
 jumps to the top of window.

I can confirm the problem.

 The problem is in the help_handle_key() function:
 case KEY_UP:
 case ALT ('\t'):
 /* select previous link */
 new_item = select_prev_link (startpoint, selected_item);

 The 'startpoint' variable should be a pointer to the first byte
 displayed in the Help window. But here it has a wrong value - that's
 the problem. That's why select_prev_link() cannot find the link and
 returns NULL, and the selection moves to the first link in the window.

 I tried to find out what's wrong with the 'startpoint' variable.
 I came to the conclusion that 'startpoint' is used here erroneously
 instead of 'currentpoint' variable.

I am not convinced of that yet - see below. It's more likely
that the navigation code messes the value of 'startpoint'.

 'currentpoint' always contains a pointer to the first byte displayed,
 and it should be used here. And by the way, 'startpoint' variable
 seems to be totally useless.

This is not true - 'currentpoint' points to the first byte of
the currently disaplyed help contents, while 'startpoint'
points to the start of the current link/topic. 'startpoint'
gets messed after one returns back from following a link.


 So in my patch I replaced 'startpoint' with 'currentpoint' and
 removed the 'startpoint' variable completely.

I won't apply this patch yet. I want to investigate further.
___
Mc-devel mailing list
http://mail.gnome.org/mailman/listinfo/mc-devel


Re: [PATCH] mc crashes when temporary directory cannot be created

2006-11-30 Thread Pavel Tsekov


On Wed, 29 Nov 2006, Leonard den Ottolander wrote:

 Hello Jindrich,

 On Tue, 2006-11-28 at 13:21 +0100, Jindrich Novy wrote:
 IMO only removal of the fallback will prevent
 the infinite loop in any case as it shouldn't call mc_mkstemps() at all.

 That cure seems worse than the disease. Isn't the real problem the fact
 that mc_mkstemps() blindly concats tmpdir to the prefix instead of
 testing if mc_tmpdir() succeeded? Why not abort mc_mkstemps() if
 mc_tmpdir() returns /dev/null/?

 What about the attached (untested) patch?

 By the way, could you please add the -p option to your diffs?

There is an even simpler cure. In mc_tmpdir() when executing
the fallback code pass an absolute path to mc_mkstemps().
This will prevent the loop. However I am not yet conviced
that this is the best solution. I am still investigating.

___
Mc-devel mailing list
http://mail.gnome.org/mailman/listinfo/mc-devel