Re: Big patch for mcview
Hello, On Wed, 6 Apr 2005, Roland Illig wrote: Roland Illig wrote: Hi all, I must have had too much time, as I have partly rewritten mcview. The patch is quite large and changes many things. However, I have tested it with all available types of data sources (see the code for explanation), and it seems to work. Here is my second try for the viewer. As noted by Jindrich, I accidentally removed an assignment to view-view_active. This has been re-added. The current patch has been working fine for me for the last few weeks. I'd like to commit it now, so that I can continue working on the remaining issues to mcview. Droping a short note that you've done the commit wouldn't hurt so much, would it ? ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Big patch for mcview
Pavel Tsekov wrote: Droping a short note that you've done the commit wouldn't hurt so much, would it ? Sorry, I forgot it. :( Committed. Roland ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Big patch for mcview
Hello, On Wed, 6 Apr 2005, Roland Illig wrote: Roland Illig wrote: Hi all, I must have had too much time, as I have partly rewritten mcview. The patch is quite large and changes many things. However, I have tested it with all available types of data sources (see the code for explanation), and it seems to work. Here is my second try for the viewer. As noted by Jindrich, I accidentally removed an assignment to view-view_active. This has been re-added. The current patch has been working fine for me for the last few weeks. I'd like to commit it now, so that I can continue working on the remaining issues to mcview. Well, it was working fine for you except that you were unable to reproduce a bug which was easily reproducable. No one on the list commented on that patch - I guess noone even installed it. Well let's hope that in this particular case silence means agreement. Maybe the next time you don't have to bother sending to the list at all. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Big patch for mcview (fwd)
Hello,
I'm forwarding this to the list (again) just to make sure that it won't be
missed because of the wrong settings of my clock.
-- Forwarded message --
Date: Mon, 28 Mar 2005 20:48:19 +0300
From: Pavel Tsekov [EMAIL PROTECTED]
To: Roland Illig [EMAIL PROTECTED]
Cc: MC dev mc-devel@gnome.org
Subject: Re: Big patch for mcview
Hello,
On Fri, 1 Apr 2005, Roland Illig wrote:
Jindrich Makovicka wrote:
Crashes when switching between viewer modes. To reproduce, use F3 on a
jpg/mp3/whatever (assuming you have mpg123, ImageMagick or other
corresponding viewer), press F8 to display raw, then press F4 = sig11.
I cannot reproduce this. Can you be more specific where mc crashes? (a
backtrace of an mc with CFLAGS=-ggdb would be most helpful)
[EMAIL PROTECTED] ~]$ gdb /home/ptsekov/mc-test/usr/bin/mc 6379
GNU gdb Red Hat Linux (6.1post-1.20040607.43rh)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you
are
welcome to change it and/or distribute copies of it under certain
conditions.
Type show copying to see the conditions.
There is absolutely no warranty for GDB. Type show warranty for
details.
This GDB was configured as i386-redhat-linux-gnu...Using host
libthread_db library /lib/tls/libthread_db.so.1.
Attaching to program: /home/ptsekov/mc-test/usr/bin/mc, process 6379
Reading symbols from /usr/lib/libgmodule-2.0.so.0...done.
Loaded symbols for /usr/lib/libgmodule-2.0.so.0
Reading symbols from /lib/libdl.so.2...done.
Loaded symbols for /lib/libdl.so.2
Reading symbols from /usr/lib/libglib-2.0.so.0...done.
Loaded symbols for /usr/lib/libglib-2.0.so.0
Reading symbols from /usr/lib/libgpm.so.1...done.
Loaded symbols for /usr/lib/libgpm.so.1
Reading symbols from /usr/lib/libslang-utf8.so.1...done.
Loaded symbols for /usr/lib/libslang-utf8.so.1
Reading symbols from /lib/libnsl.so.1...done.
Loaded symbols for /lib/libnsl.so.1
Reading symbols from /lib/tls/libc.so.6...done.
Loaded symbols for /lib/tls/libc.so.6
Reading symbols from /lib/ld-linux.so.2...done.
Loaded symbols for /lib/ld-linux.so.2
Reading symbols from /lib/tls/libm.so.6...done.
Loaded symbols for /lib/tls/libm.so.6
Reading symbols from /lib/libnss_files.so.2...done.
Loaded symbols for /lib/libnss_files.so.2
Reading symbols from /usr/X11R6/lib/libX11.so...done.
Loaded symbols for /usr/X11R6/lib/libX11.so
Reading symbols from /usr/lib/gconv/ISO8859-1.so...done.
Loaded symbols for /usr/lib/gconv/ISO8859-1.so
0x00eb57a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
(gdb) c
Continuing.
Detaching after fork from child process 6388.
Detaching after fork from child process 6389.
Program received signal SIGSEGV, Segmentation fault.
0x080b68ba in mc_read (handle=0, buffer=0x8633219 , count=7671) at
../../mc/vfs/vfs.c:392
392 MC_HANDLEOP(read, (int handle, char *buffer, int count), (vfs_info
(handle), buffer, count) )
(gdb) bt
#0 0x080b68ba in mc_read (handle=0, buffer=0x8633219 , count=7671) at
../../mc/vfs/vfs.c:392
#1 0x080906bc in view_growbuf_read_until (view=0x861d430, ofs=4294967295)
at ../../mc/src/view.c:309
#2 0x08092cf0 in get_bottom_first (view=0x861d430, do_not_cache=1,
really=1) at ../../mc/src/view.c:1296
#3 0x08094145 in toggle_hex_mode (view=0x861d430) at
../../mc/src/view.c:1930
#4 0x0809a733 in buttonbar_callback (bb=0x8632eb0, msg=5, parm=1004) at
../../mc/src/widget.c:2244
#5 0x0806053a in dlg_try_hotkey (h=0x861d598, d_key=1004) at
../../mc/src/dialog.c:626
#6 0x08060618 in dlg_key_event (h=0x861d598, d_key=1004) at
../../mc/src/dialog.c:664
#7 0x08060946 in dlg_process_event (h=0x861d598, key=1004,
event=0xbfef3e50) at ../../mc/src/dialog.c:765
#8 0x08060b40 in frontend_run_dlg (h=0x861d598) at
../../mc/src/dialog.c:797
#9 0x08060a54 in run_dlg (h=0x861d598) at ../../mc/src/dialog.c:812
#10 0x0809583d in view (_command=0x861d5d8 /bin/sh
/tmp/mc-ptsekov/mcextJBI8Ca, _file=0x861d370 OggPlay-S60-MMF-1.5.1.zip,
move_dir_p=0xbfef454c, start_line=0) at ../../mc/src/view.c:2574
#11 0x080638c5 in exec_extension (filename=0x861d370
OggPlay-S60-MMF-1.5.1.zip,
data=0x8631763 \n\n# zoo\nregex/\\.(zoo|ZOO)$\n\tOpen=%cd
%p#uzoo\n\tView=%view{ascii} zoo l %f\n\t\n# lha\ntype/^LHa\\
.*archive\n\tOpen=%cd %p#ulha\n\tView=%view{ascii} lha l %f\n\n#
arj\nregex/\\.a(rj|[0-9][0-9])$\n\tOpen=%cd %p#uarj\n\t...,
move_dir=0xbfef454c,
start_line=0) at ../../mc/src/ext.c:238
#12 0x0806447a in regex_command (filename=0x861dab0
OggPlay-S60-MMF-1.5.1.zip, action=0xbfef4500 View,
move_dir=0xbfef454c)
at ../../mc/src/ext.c:594
#13 0x08059b02 in view_file_at_line (filename=0x861dab0
OggPlay-S60-MMF-1.5.1.zip, plain_view=0, internal=1, start_line=0)
at ../../mc/src/cmd.c:124
#14 0x08059b8a in view_file (filename=0x861dab0
OggPlay-S60-MMF-1.5.1.zip, plain_view=0, internal=1) at
../../mc/src/cmd.c:150
#15 0x08059d8c in do_view_cmd (normal=0) at ../../mc/src/cmd.c:208
#16 0x08059dc5 in view_cmd () at ../../mc/src/cmd.c:219
#17 0x0809a733
Re: Big patch for mcview (fwd)
Pavel Tsekov wrote: Hello, I'm forwarding this to the list (again) just to make sure that it won't be missed because of the wrong settings of my clock. I have already resolved the issue with Roland - the patch accidentally removed the line view-view_active = 1; which then caused the crash. After returning this line it works fine. Regards, -- Jindrich Makovicka ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Big patch for mcview
Hello,
On Fri, 1 Apr 2005, Roland Illig wrote:
Jindrich Makovicka wrote:
Crashes when switching between viewer modes. To reproduce, use F3 on a
jpg/mp3/whatever (assuming you have mpg123, ImageMagick or other
corresponding viewer), press F8 to display raw, then press F4 = sig11.
I cannot reproduce this. Can you be more specific where mc crashes? (a
backtrace of an mc with CFLAGS=-ggdb would be most helpful)
[EMAIL PROTECTED] ~]$ gdb /home/ptsekov/mc-test/usr/bin/mc 6379
GNU gdb Red Hat Linux (6.1post-1.20040607.43rh)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you
are
welcome to change it and/or distribute copies of it under certain
conditions.
Type show copying to see the conditions.
There is absolutely no warranty for GDB. Type show warranty for
details.
This GDB was configured as i386-redhat-linux-gnu...Using host
libthread_db library /lib/tls/libthread_db.so.1.
Attaching to program: /home/ptsekov/mc-test/usr/bin/mc, process 6379
Reading symbols from /usr/lib/libgmodule-2.0.so.0...done.
Loaded symbols for /usr/lib/libgmodule-2.0.so.0
Reading symbols from /lib/libdl.so.2...done.
Loaded symbols for /lib/libdl.so.2
Reading symbols from /usr/lib/libglib-2.0.so.0...done.
Loaded symbols for /usr/lib/libglib-2.0.so.0
Reading symbols from /usr/lib/libgpm.so.1...done.
Loaded symbols for /usr/lib/libgpm.so.1
Reading symbols from /usr/lib/libslang-utf8.so.1...done.
Loaded symbols for /usr/lib/libslang-utf8.so.1
Reading symbols from /lib/libnsl.so.1...done.
Loaded symbols for /lib/libnsl.so.1
Reading symbols from /lib/tls/libc.so.6...done.
Loaded symbols for /lib/tls/libc.so.6
Reading symbols from /lib/ld-linux.so.2...done.
Loaded symbols for /lib/ld-linux.so.2
Reading symbols from /lib/tls/libm.so.6...done.
Loaded symbols for /lib/tls/libm.so.6
Reading symbols from /lib/libnss_files.so.2...done.
Loaded symbols for /lib/libnss_files.so.2
Reading symbols from /usr/X11R6/lib/libX11.so...done.
Loaded symbols for /usr/X11R6/lib/libX11.so
Reading symbols from /usr/lib/gconv/ISO8859-1.so...done.
Loaded symbols for /usr/lib/gconv/ISO8859-1.so
0x00eb57a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
(gdb) c
Continuing.
Detaching after fork from child process 6388.
Detaching after fork from child process 6389.
Program received signal SIGSEGV, Segmentation fault.
0x080b68ba in mc_read (handle=0, buffer=0x8633219 , count=7671) at
../../mc/vfs/vfs.c:392
392 MC_HANDLEOP(read, (int handle, char *buffer, int count), (vfs_info
(handle), buffer, count) )
(gdb) bt
#0 0x080b68ba in mc_read (handle=0, buffer=0x8633219 , count=7671) at
../../mc/vfs/vfs.c:392
#1 0x080906bc in view_growbuf_read_until (view=0x861d430, ofs=4294967295)
at ../../mc/src/view.c:309
#2 0x08092cf0 in get_bottom_first (view=0x861d430, do_not_cache=1,
really=1) at ../../mc/src/view.c:1296
#3 0x08094145 in toggle_hex_mode (view=0x861d430) at
../../mc/src/view.c:1930
#4 0x0809a733 in buttonbar_callback (bb=0x8632eb0, msg=5, parm=1004) at
../../mc/src/widget.c:2244
#5 0x0806053a in dlg_try_hotkey (h=0x861d598, d_key=1004) at
../../mc/src/dialog.c:626
#6 0x08060618 in dlg_key_event (h=0x861d598, d_key=1004) at
../../mc/src/dialog.c:664
#7 0x08060946 in dlg_process_event (h=0x861d598, key=1004,
event=0xbfef3e50) at ../../mc/src/dialog.c:765
#8 0x08060b40 in frontend_run_dlg (h=0x861d598) at
../../mc/src/dialog.c:797
#9 0x08060a54 in run_dlg (h=0x861d598) at ../../mc/src/dialog.c:812
#10 0x0809583d in view (_command=0x861d5d8 /bin/sh
/tmp/mc-ptsekov/mcextJBI8Ca, _file=0x861d370 OggPlay-S60-MMF-1.5.1.zip,
move_dir_p=0xbfef454c, start_line=0) at ../../mc/src/view.c:2574
#11 0x080638c5 in exec_extension (filename=0x861d370
OggPlay-S60-MMF-1.5.1.zip,
data=0x8631763 \n\n# zoo\nregex/\\.(zoo|ZOO)$\n\tOpen=%cd
%p#uzoo\n\tView=%view{ascii} zoo l %f\n\t\n# lha\ntype/^LHa\\
.*archive\n\tOpen=%cd %p#ulha\n\tView=%view{ascii} lha l %f\n\n#
arj\nregex/\\.a(rj|[0-9][0-9])$\n\tOpen=%cd %p#uarj\n\t...,
move_dir=0xbfef454c,
start_line=0) at ../../mc/src/ext.c:238
#12 0x0806447a in regex_command (filename=0x861dab0
OggPlay-S60-MMF-1.5.1.zip, action=0xbfef4500 View,
move_dir=0xbfef454c)
at ../../mc/src/ext.c:594
#13 0x08059b02 in view_file_at_line (filename=0x861dab0
OggPlay-S60-MMF-1.5.1.zip, plain_view=0, internal=1, start_line=0)
at ../../mc/src/cmd.c:124
#14 0x08059b8a in view_file (filename=0x861dab0
OggPlay-S60-MMF-1.5.1.zip, plain_view=0, internal=1) at
../../mc/src/cmd.c:150
#15 0x08059d8c in do_view_cmd (normal=0) at ../../mc/src/cmd.c:208
#16 0x08059dc5 in view_cmd () at ../../mc/src/cmd.c:219
#17 0x0809a733 in buttonbar_callback (bb=0x861ae30, msg=5, parm=1003) at
../../mc/src/widget.c:2244
#18 0x0806053a in dlg_try_hotkey (h=0x8600460, d_key=1003) at
../../mc/src/dialog.c:626
#19 0x08060618 in dlg_key_event (h=0x8600460, d_key=1003) at
../../mc/src/dialog.c:664
#20 0x08060946 in dlg_process_event (h=0x8600460, key=1003,
event=0xbfef46a0) at
Re: Big patch for mcview
Roland Illig wrote: Pavel Tsekov wrote: First of all - this patch could have been much smaller and thus easier to review/understand. 25 % (line 666 to the end) of the patch are hunks which do the following: get_byte = view-get_byte Well, simply keeping get_byte () and calling view-get_byte from within would have been much nicer. Also you could have made our lives easier if you have moved most of the new functions that you have introduced to the end of the file - this way it would be much easier to read the patch. Sorry for these. I had uploaded an old version of the patch. An improved patch is available: http://www.roland-illig.de/tmp/viewer-try2.patch Crashes when switching between viewer modes. To reproduce, use F3 on a jpg/mp3/whatever (assuming you have mpg123, ImageMagick or other corresponding viewer), press F8 to display raw, then press F4 = sig11. Regards, -- Jindrich Makovicka ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Big patch for mcview
Pavel Tsekov wrote: First of all - this patch could have been much smaller and thus easier to review/understand. 25 % (line 666 to the end) of the patch are hunks which do the following: get_byte = view-get_byte Well, simply keeping get_byte () and calling view-get_byte from within would have been much nicer. Also you could have made our lives easier if you have moved most of the new functions that you have introduced to the end of the file - this way it would be much easier to read the patch. Sorry for these. I had uploaded an old version of the patch. An improved patch is available: http://www.roland-illig.de/tmp/viewer-try2.patch * Grouped all variables that have to do with the data source management. Good - also you've doubled the number of variables (7 vs 12). Not that it matters so much but I'd expect something much cleaner to represent the so called data source. In my imagination it is something like a struct with common methods exported much like, say, the vfs. Then you have several datasource objects and a pointer in the WView being set to point the one that is currently necessary. That would have made the source code even larger. I could invent a union viewer_datasource for this. I already explained the increase in the number of variables because there's no double-triple-use of them, like it was before. See below. Also, this is not the final patch for mcview. It's just something that makes it more understandable---in my opinion and at least to me. * Made every variable have only one purpose. (Before, you could never be sure what the view-data field contained, as it was used for keeping the error message as well as the mmapped file and the data of the non-mmapped file.) Good but see above. Also, I always have to think what was the difference between `ds_file_size' and `ds_file_datasize'. Would it help to name them ds_file_filesize and ds_file_datasize? Or do you want even other names? * Provided a clean interface for the get_byte() function, as well as four different implementations of it. It makes sense in the light of what you're trying to accomplish. One note though: view_set_datasource_none () view_set_datasource_string () view_set_datasource_file () init_growing_view () It was not so easy for me to derive the name of the function initializing the view to work with pipes. The init_growing_view is the legacy interface to WView. I'm planning to remove it soon. But the patch had been big enough for now. :) Perhaps you may worry about the fact that my patch makes the file 119 lines longer, but I hope the code becomes more readable and maintainable. Personally, I don't worry so much about the number of lines in the file. I think in the past you were the one complaining that view.c was too large. My major concern is that this code although announced as something quite new consists mainly of cosmetic changes (IMHO). I'd like others to comment too though. These cosmetic changes do not add much functionality, but they bring in some common structure to all datasources, which helps to understand the concept of the datasource (interface, and implementation). That's all I want. It also makes it easier to re-add the mmap datasource, if we should ever need to do so. Roland ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: Big patch for mcview
On Wednesday 02 March 2005 21:40, Roland Illig wrote: Hi all, I must have had too much time, as I have partly rewritten mcview. The patch is quite large and changes many things. However, I have tested it with all available types of data sources (see the code for explanation), and it seems to work. Gives Broken pipe warning when closing tarball WBR Dmitry ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
