From: Todd Z. <tm...@po...> - 2006-05-31 23:10:12
Attachments:
gtkpod.backtrace
|
Hi again, I've got a nice segfault with my video ipod (model A147). Testing with an older ipod (model 9282) works well. I did a complete wipe and restore on the video ipod. I add a directory of mp3's with a folder.jpg and when saving the changes gtkpod segfaults after copying the tracks, as it's writing the database. If i add the same directory without the folder.jpg, all is well. I tried to track this down but it's just not clicking for me where the problem is. A backtrace is attached, hopefully it'll be more obvious to someone who knows what to look for. This is CVS from earlier this afternoon for both libgpod and gtkpod. Thanks, -- Todd OpenPGP -> KeyID: 0xD654075A | URL: www.pobox.com/~tmz/pgp ====================================================================== A government big enough to give you everything you want is a government big enough to take from you everything you have. -- Gerald R. Ford, 1976 |
From: Jorg S. <Jor...@gm...> - 2006-06-01 01:22:15
|
> I've got a nice segfault with my video ipod (model A147). Testing > with an older ipod (model 9282) works well. > > I did a complete wipe and restore on the video ipod. I add a > directory of mp3's with a folder.jpg and when saving the changes > gtkpod segfaults after copying the tracks, as it's writing the > database. If i add the same directory without the folder.jpg, all is > well. > > This is CVS from earlier this afternoon for both libgpod and gtkpod. > ------------------------------------------------------------------------ > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread -1209002288 (LWP 28810)] > 0x4836d57e in g_hash_table_lookup () from /usr/lib/libglib-2.0.so.0 > (gdb) bt > #0 0x4836d57e in g_hash_table_lookup () from /usr/lib/libglib-2.0.so.0 > #1 0x00e09393 in itdb_device_get_sysinfo (device=0x947f608, field=0xe09f7b "ModelNumStr") at itdb_device.c:372 > #2 0x00e09409 in itdb_device_get_ipod_model (device=0x947f608) at itdb_device.c:383 > #3 0x00e094e1 in itdb_device_get_artwork_formats (device=0x947f608) at itdb_device.c:414 > #4 0x00e070d3 in write_mhsd (db=0xbfb23234, buffer=0x94916b8, type=Variable "type" is not available. > ) at db-artwork-writer.c:456 > #5 0x00e07bf4 in write_mhfd (db=0xbfb23234, buffer=0x9492fd8, id_max=Variable "id_max" is not available. > ) at db-artwork-writer.c:950 > #6 0x00e07f12 in ipod_write_artwork_db (itdb=0x947f5d0) at db-artwork-writer.c:1019 What is that "Variable... is not available" about? It crases in g_hash_table_lookup() can you send us a copy of your SysInfo file (at the time when it crashes)? I'm pretty much clueless too... Cheers, JCS. |
From: Todd Z. <tm...@po...> - 2006-06-01 02:45:19
Attachments:
SysInfo
gtkpod.valgind
|
Hi Jorg, Jorg Schuler wrote: >> Program received signal SIGSEGV, Segmentation fault. >> [Switching to Thread -1209002288 (LWP 28810)] >> 0x4836d57e in g_hash_table_lookup () from /usr/lib/libglib-2.0.so.0 >> (gdb) bt >> #0 0x4836d57e in g_hash_table_lookup () from /usr/lib/libglib-2.0.so.0 >> #1 0x00e09393 in itdb_device_get_sysinfo (device=0x947f608, field=0xe09f7b "ModelNumStr") at itdb_device.c:372 >> #2 0x00e09409 in itdb_device_get_ipod_model (device=0x947f608) at itdb_device.c:383 >> #3 0x00e094e1 in itdb_device_get_artwork_formats (device=0x947f608) at itdb_device.c:414 >> #4 0x00e070d3 in write_mhsd (db=0xbfb23234, buffer=0x94916b8, type=Variable "type" is not available. >> ) at db-artwork-writer.c:456 >> #5 0x00e07bf4 in write_mhfd (db=0xbfb23234, buffer=0x9492fd8, id_max=Variable "id_max" is not available. >> ) at db-artwork-writer.c:950 >> #6 0x00e07f12 in ipod_write_artwork_db (itdb=0x947f5d0) at db-artwork-writer.c:1019 > > What is that "Variable... is not available" about? You've got me. I do know from tossing some printf's about that both type and id_max are available in write_mhsd(). > It crases in g_hash_table_lookup() can you send us a copy of your > SysInfo file (at the time when it crashes)? Sure. It's attached. Seems fine, doesn't it? In my poking it appears that the model is returned correctly. It matches in itdb_device_get_ipod_model() and &ipod_model_table[50] is what's returned to itdb_device_get_artwork_formats(). I added a printf just before and after line 456 in db-artwork-writer.c and only the first one is printed. I'm not sure if that helps you or not, but it doesn't leave many steps between the last successful printf I tossed into itdb_device.c at line 417 and then not getting passed the "format = " line in db-artwork-writer.c. > I'm pretty much clueless too... That's not good. :) Also attached is the output of valgrind, in the hopes that it helps illuminate the problem better. Thanks, -- Todd OpenPGP -> KeyID: 0xD654075A | URL: www.pobox.com/~tmz/pgp ====================================================================== It's a little childish and stupid, but then, so is high school. -- Ferris Bueller's Day Off |
From: Todd Z. <tm...@po...> - 2006-06-01 04:55:56
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 I wrote: > Jorg Schuler wrote: >> What is that "Variable... is not available" about? > > You've got me. I should know better than to not ask my friend Google. Apparently that happens if gdb can't get the variable due to optimizations that the compiler does. I recompiled libgpod with -O0 -g3 and now those messages are gone. But the segfault remains, unfortunately. I also installed the glib and gtk2 debug symbol packages (I had forgotten to load them when I installed FC5). Here is another backtrace. I'm not sure it will help any more than the previous one, but it can't hurt. Let me know if you have other ideas or things I should try to debug this. Could I just hardcode the proper value for format that itdb_device_get_artwork_formats() is returning, just to take that function out of the loop? Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1208899888 (LWP 25690)] 0x4836d57e in IA__g_hash_table_lookup (hash_table=0x50692f44, key=0x8a2a0a) at ghash.c:242 242 node = &hash_table->nodes (gdb) bt #0 0x4836d57e in IA__g_hash_table_lookup (hash_table=0x50692f44, key=0x8a2a0a) at ghash.c:242 #1 0x0089f20a in itdb_device_get_sysinfo (device=0xa667600, field=0x8a2a0a "ModelNumStr") at itdb_device.c:372 #2 0x0089f245 in itdb_device_get_ipod_model (device=0xa667600) at itdb_device.c:383 #3 0x0089f36f in itdb_device_get_artwork_formats (device=0xa667600) at itdb_device.c:414 #4 0x0089c0da in write_mhni (db=0xbf93e908, thumb=0xa6de740, correlation_id=1028, buffer=0xa6e4be0) at db-artwork-writer.c:456 #5 0x0089c2e7 in write_mhod (db=0xbf93e908, thumb=0xa6de740, correlation_id=1028, buffer=0xa6e4bc8) at db-artwork-writer.c:512 #6 0x0089c5c8 in write_mhii (db=0xbf93e908, data=0xa6dea38, buffer=0xa6e4bb0) at db-artwork-writer.c:581 #7 0x0089c9e5 in write_mhli (db=0xbf93e908, buffer=0xa6e31d8) at db-artwork-writer.c:639 #8 0x0089d207 in write_mhsd (db=0xbf93e908, buffer=0xa6dc970, type=MHSD_TYPE_MHLI) at db-artwork-writer.c:897 #9 0x0089d3b9 in write_mhfd (db=0xbf93e908, buffer=0x9b2f860, id_max=73) at db-artwork-writer.c:950 #10 0x0089d556 in ipod_write_artwork_db (itdb=0xa6675c8) at db-artwork-writer.c:1019 #11 0x00890402 in itdb_write_file (itdb=0xa6675c8, filename=0xa666a98 "/media/ZAPHOD/iPod_Control/iTunes/iTunesDB", error=0xbf93ea04) at itdb_itunesdb.c:4219 #12 0x0089066b in itdb_write (itdb=0xa6675c8, error=0xbf93ea04) at itdb_itunesdb.c:4308 #13 0x08083565 in gp_save_itdb (itdb=0xa6675c8) at file_itunesdb.c:1603 #14 0x08084088 in handle_export () at file_itunesdb.c:1775 #15 0x483fe1c9 in IA__g_cclosure_marshal_VOID__VOID (closure=0x983e130, return_value=0x0, n_param_values=1, param_values=0xbf93ec6c, invocation_hint=0xbf93eb7c, marshal_data=0x8066b00) at gmarshal.c:77 #16 0x483f0f6d in IA__g_closure_invoke (closure=0x983e130, return_value=0x0, n_param_values=1, param_values=0xbf93ec6c, invocation_hint=0xbf93eb7c) at gclosure.c:490 #17 0x48401a3d in signal_emit_unlocked_R (node=0x9855558, detail=0, instance=0x9856aa8, emission_return=0x0, instance_and_params=0xbf93ec6c) at gsignal.c:2438 #18 0x48402f47 in IA__g_signal_emit_valist (instance=0x9856aa8, signal_id=136, detail=0, var_args=0xbf93eeb8 "=�") at gsignal.c:2197 #19 0x48404d7e in IA__g_signal_emit_by_name (instance=0x9856aa8, detailed_signal=0x488b0f6b "clicked") at gsignal.c:2265 #20 0x4883c587 in button_clicked (widget=0x97a5e58, button=0x9856aa8) at gtktoolbutton.c:645 #21 0x483fe1c9 in IA__g_cclosure_marshal_VOID__VOID (closure=0x9855500, return_value=0x0, n_param_values=1, param_values=0xbf93f0fc, invocation_hint=0xbf93f00c, marshal_data=0x4883c560) at gmarshal.c:77 #22 0x483f0f6d in IA__g_closure_invoke (closure=0x9855500, return_value=0x0, n_param_values=1, param_values=0xbf93f0fc, invocation_hint=0xbf93f00c) at gclosure.c:490 #23 0x48401a3d in signal_emit_unlocked_R (node=0x9854b68, detail=0, instance=0x97a5e58, emission_return=0x0, instance_and_params=0xbf93f0fc) at gsignal.c:2438 #24 0x48402f47 in IA__g_signal_emit_valist (instance=0x97a5e58, signal_id=128, detail=0, var_args=0xbf93f33c "\212clH�fBHX^z\tX�\223��|lHX^z\tp|lHx�\223���?HX^z\t�\200z\t=�") at gsignal.c:2197 #25 0x48403109 in IA__g_signal_emit (instance=0x97a5e58, signal_id=128, detail=0) at gsignal.c:2241 #26 0x486c63d3 in IA__gtk_button_clicked (button=0x97a5e58) at gtkbutton.c:845 #27 0x486c7cae in gtk_real_button_released (button=0x97a5e58) at gtkbutton.c:1380 #28 0x483fe1c9 in IA__g_cclosure_marshal_VOID__VOID (closure=0x9854af0, return_value=0x0, n_param_values=1, param_values=0xbf93f5bc, invocation_hint=0xbf93f4cc, marshal_data=0x486c7c70) at gmarshal.c:77 #29 0x483ef7a9 in g_type_class_meta_marshal (closure=0x9854af0, return_value=0x0, n_param_values=1, param_values=0xbf93f5bc, invocation_hint=0xbf93f4cc, marshal_data=0x1a4) at gclosure.c:567 #30 0x483f0f6d in IA__g_closure_invoke (closure=0x9854af0, return_value=0x0, n_param_values=1, param_values=0xbf93f5bc, invocation_hint=0xbf93f4cc) - ---Type <return> to continue, or q <return> to quit--- at gclosure.c:490 #31 0x48401eca in signal_emit_unlocked_R (node=0x9854b28, detail=0, instance=0x97a5e58, emission_return=0x0, instance_and_params=0xbf93f5bc) at gsignal.c:2368 #32 0x48402f47 in IA__g_signal_emit_valist (instance=0x97a5e58, signal_id=127, detail=0, var_args=0xbf93f7fc "\032dlHxC\231H�dlH\030�\223��dlHX^z\t�dlH8�\223�.5yHX^z\t@&z\t\230zz\t�fBH|�\223���}\th�\223���>H��}\t��\223�\002") at gsignal.c:2197 #33 0x48403109 in IA__g_signal_emit (instance=0x97a5e58, signal_id=127, detail=0) at gsignal.c:2241 #34 0x486c6463 in IA__gtk_button_released (button=0x97a5e58) at gtkbutton.c:837 #35 0x486c64c1 in gtk_button_button_release (widget=0x97a5e58, event=0x97a2640) at gtkbutton.c:1273 #36 0x4879352e in _gtk_marshal_BOOLEAN__BOXED (closure=0x97dfbe8, return_value=0xbf93f9a0, n_param_values=2, param_values=0xbf93fa7c, invocation_hint=0xbf93f98c, marshal_data=0x486c64a0) at gtkmarshalers.c:83 #37 0x483ef7a9 in g_type_class_meta_marshal (closure=0x97dfbe8, return_value=0xbf93f9a0, n_param_values=2, param_values=0xbf93fa7c, invocation_hint=0xbf93f98c, marshal_data=0xb4) at gclosure.c:567 #38 0x483f0f6d in IA__g_closure_invoke (closure=0x97dfbe8, return_value=0xbf93f9a0, n_param_values=2, param_values=0xbf93fa7c, invocation_hint=0xbf93f98c) at gclosure.c:490 #39 0x48402083 in signal_emit_unlocked_R (node=0x97def10, detail=0, instance=0x97a5e58, emission_return=0xbf93fc3c, instance_and_params=0xbf93fa7c) at gsignal.c:2476 #40 0x48402d0f in IA__g_signal_emit_valist (instance=0x97a5e58, signal_id=28, detail=0, var_args=Variable "var_args" is not available. ) at gsignal.c:2207 #41 0x48403109 in IA__g_signal_emit (instance=0x97a5e58, signal_id=28, detail=0) at gsignal.c:2241 #42 0x4887e7e8 in gtk_widget_event_internal (widget=0x97a5e58, event=0x97a2640) at gtkwidget.c:3751 #43 0x4878cf03 in IA__gtk_propagate_event (widget=0x97a5e58, event=0x97a2640) at gtkmain.c:2195 #44 0x4878e157 in IA__gtk_main_do_event (event=0x97a2640) at gtkmain.c:1424 #45 0x4856d93a in gdk_event_dispatch (source=0x97a5a18, callback=0, user_data=0x0) at gdkevents-x11.c:2291 #46 0x4837915d in IA__g_main_context_dispatch (context=0x97a5a60) at gmain.c:1916 #47 0x4837c3ef in g_main_context_iterate (context=0x97a5a60, block=1, dispatch=1, self=0x9789040) at gmain.c:2547 #48 0x4837c799 in IA__g_main_loop_run (loop=0xa6c7e98) at gmain.c:2751 #49 0x4878e5d4 in IA__gtk_main () at gtkmain.c:1003 #50 0x08088ef5 in main (argc=1, argv=0xbf93ff94) at main.c:143 (gdb) - -- Todd OpenPGP -> KeyID: 0xD654075A | URL: www.pobox.com/~tmz/pgp ====================================================================== Duct tape is like the Force. It has a light side, a dark side, and it holds the universe together.... -- Carl Zwanzig -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.3 (GNU/Linux) Comment: When crypto is outlawed bayl bhgynjf jvyy unir cevinpl. iG0EARECAC0FAkR+chMmGGh0dHA6Ly93d3cucG9ib3guY29tL350bXovcGdwL3Rt ei5hc2MACgkQuv+09NZUB1oKHQCgm5YmAyElfgpaGnrNlh3CSGh2JGcAnjWuEUlR xStIb86pU9OWVyEqzJ2B =Y4Sb -----END PGP SIGNATURE----- |
From: Todd Z. <tm...@po...> - 2006-06-01 08:09:07
Attachments:
libgpod-artwork-segfault.patch
|
Hi Jorg, I can't say that I really understand the why behind this, but I changed db->db.photodb->device to db->db.itdb->device in the call to itdb_device_get_artwork_formats and that has made the segfault vanish. The Itdb_DB struct is a union of Itdb_PhotoDB and Itdb_iTunesDB, both of which contain Itdb_Device, so I would have (naively) guessed that either one would work the same. This fix isn't correct, as with it I get a similar segfault when running test-photos. But hopefully this will help narrow down the real problem. -- Todd OpenPGP -> KeyID: 0xD654075A | URL: www.pobox.com/~tmz/pgp ====================================================================== Government of man by man in every form is oppression. -- Pierre Joseph |
From: Christophe F. <te...@gn...> - 2006-06-01 08:22:36
|
Le jeudi 01 juin 2006 =C3=A0 04:08 -0400, Todd Zullinger a =C3=A9crit : > Hi Jorg, >=20 > I can't say that I really understand the why behind this, but I > changed db->db.photodb->device to db->db.itdb->device in the call to > itdb_device_get_artwork_formats and that has made the segfault vanish. >=20 > The Itdb_DB struct is a union of Itdb_PhotoDB and Itdb_iTunesDB, both > of which contain Itdb_Device, so I would have (naively) guessed that > either one would work the same. Hmmm, I hadn't looked at the way the database work now, but this union with the ItdbDevice field, and all the userdata stuff not being at the same offset in the itunesdb and photodb structures seems error-prone :-/ Maybe we could have something like that instead:=20 struct _Itdb_DB { DBType db_type; GList *items; GList *playlists; Itdb_Device *device;/* iPod device info */ /* below is for use by application */ guint64 usertype; gpointer userdata; /* function called to duplicate userdata */ ItdbUserDataDuplicateFunc userdata_duplicate; /* function called to free userdata */ ItdbUserDataDestroyFunc userdata_destroy; union { Itdb_PhotoDB *photodb; Itdb_iTunesDB *itdb; } } struct _Itdb_PhotoDB { }; struct _Itdb_iTunesDB { gchar *filename; /* filename of iTunesDB */ guint32 version; guint64 id; }; at least, the memory layout of the common portion would match. Though it might make the code uglier, or not be practical, I haven't looked how it's used, this is just some random thinking. Christophe |
From: Christophe F. <te...@gn...> - 2006-06-01 09:59:49
|
Le jeudi 01 juin 2006 =C3=A0 10:22 +0200, Christophe Fergeau a =C3=A9crit= : >=20 > Maybe we could have something like that instead:=20 >=20 > struct _Itdb_DB > { > DBType db_type; > Itdb_Device *device;/* iPod device info */ > /* below is for use by application */ > guint64 usertype; > gpointer userdata; > /* function called to duplicate userdata */ > ItdbUserDataDuplicateFunc userdata_duplicate; > /* function called to free userdata */ > ItdbUserDataDestroyFunc userdata_destroy; > union { > Itdb_PhotoDB *photodb; > Itdb_iTunesDB *itdb; > } > } >=20 > struct _Itdb_PhotoDB { GList *photos; GList *photoalbums; =20 > }; >=20 > struct _Itdb_iTunesDB { GList *tracks; GList *playlists; > gchar *filename; /* filename of iTunesDB */ > guint32 version; > guint64 id; > }; is probably better Christophe |
From: Christophe F. <te...@gn...> - 2006-06-01 08:29:37
|
Le jeudi 01 juin 2006 =C3=A0 04:08 -0400, Todd Zullinger a =C3=A9crit : > This fix isn't correct, as with it I get a similar segfault when > running test-photos. But hopefully this will help narrow down the > real problem. Does that help if you change the definition of itdb.h:struct _Itdb_iTunesDB to: struct _Itdb_iTunesDB { GList *tracks; GList *playlists; Itdb_Device *device;/* iPod device info */ /* below is for use by application */ guint64 usertype; gpointer userdata; /* function called to duplicate userdata */ ItdbUserDataDuplicateFunc userdata_duplicate; /* function called to free userdata */ ItdbUserDataDestroyFunc userdata_destroy; gchar *filename; /* filename of iTunesDB */ guint32 version; guint64 id; }; (simply reorders some fields, might cause random breakage ;) Christophe |
From: Todd Z. <tm...@po...> - 2006-06-01 09:15:11
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Christophe, Christophe Fergeau wrote: > Le jeudi 01 juin 2006 à 04:08 -0400, Todd Zullinger a écrit : >> This fix isn't correct, as with it I get a similar segfault when >> running test-photos. But hopefully this will help narrow down the >> real problem. > > Does that help if you change the definition of itdb.h:struct I will give that a try when I wake up. Unless you come up with a better idea before then. It's time now to grab a drink and watch the sun rise before bed. :) > (simply reorders some fields, might cause random breakage ;) Random as in it will require some good testing to expose the breakage or as in odd things will likely break? Just so I know whether I know how much and how long I should beat on it before I say it seems to work. Thanks, - -- Todd OpenPGP -> KeyID: 0xD654075A | URL: www.pobox.com/~tmz/pgp ====================================================================== I am not young enough to know everything. -- Oscar Wilde (1854-1900) -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.3 (GNU/Linux) Comment: When crypto is outlawed bayl bhgynjf jvyy unir cevinpl. iG0EARECAC0FAkR+r8YmGGh0dHA6Ly93d3cucG9ib3guY29tL350bXovcGdwL3Rt ei5hc2MACgkQuv+09NZUB1rcWQCg6FkAkZODqEo8zzKbQxaGi2oTuN0AnjmSntKo kPxO/jtHZ44XAmnELa5r =qWgu -----END PGP SIGNATURE----- |
From: Christophe F. <te...@gn...> - 2006-06-01 10:00:21
|
Le jeudi 01 juin 2006 =C3=A0 05:13 -0400, Todd Zullinger a =C3=A9crit : > Random as in it will require some good testing to expose the breakage > or as in odd things will likely break? Just so I know whether I know > how much and how long I should beat on it before I say it seems to > work. Some stuff might subtly break, but in that case I'd be surprised if it happened. Christophe |
From: Jorg S. <Jor...@gm...> - 2006-06-01 13:10:28
|
Todd Zullinger wrote: > Hi Jorg, > > I can't say that I really understand the why behind this, but I > changed db->db.photodb->device to db->db.itdb->device in the call to > itdb_device_get_artwork_formats and that has made the segfault vanish. Good catch. Patch should be: Index: src/db-artwork-writer.c =================================================================== RCS file: /cvsroot/gtkpod/libgpod/src/db-artwork-writer.c,v retrieving revision 1.17 diff -u -r1.17 db-artwork-writer.c --- src/db-artwork-writer.c 30 May 2006 14:09:43 -0000 1.17 +++ src/db-artwork-writer.c 1 Jun 2006 13:09:27 -0000 @@ -453,7 +453,8 @@ mhni->ithmb_offset = get_gint32 (thumb->offset, buffer->byte_order); /* Work out the image padding */ - format = itdb_device_get_artwork_formats (db->db.photodb->device); + format = itdb_device_get_artwork_formats (db_get_device (db)); + g_return_val_if_fail (format, 0); while( format->type != thumb->type && format->type != -1 ) format++; mhni->vertical_padding = get_gint16 ( > The Itdb_DB struct is a union of Itdb_PhotoDB and Itdb_iTunesDB, both > of which contain Itdb_Device, so I would have (naively) guessed that > either one would work the same. Nope -- because the position of ->device is different in the two structures. Cheers, JCS. |
From: Todd Z. <tm...@po...> - 2006-06-01 22:49:10
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Jorg Schuler wrote: > Good catch. Patch should be: Cool. I did a fresh checkout with those changes included and adding cover art and photos is working well again. Thanks for the quick fix! >> The Itdb_DB struct is a union of Itdb_PhotoDB and Itdb_iTunesDB, >> both of which contain Itdb_Device, so I would have (naively) >> guessed that either one would work the same. > > Nope -- because the position of ->device is different in the two > structures. Thanks to you and Christophe for the explanations. - -- Todd OpenPGP -> KeyID: 0xD654075A | URL: www.pobox.com/~tmz/pgp ====================================================================== I honor my personality flaws for without them I would have no personality at all. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.3 (GNU/Linux) Comment: When crypto is outlawed bayl bhgynjf jvyy unir cevinpl. iG0EARECAC0FAkR/bt4mGGh0dHA6Ly93d3cucG9ib3guY29tL350bXovcGdwL3Rt ei5hc2MACgkQuv+09NZUB1psBwCg6pZoEbNlWVQ6nOZEaZHJXEa2T5oAn0ASz1cZ Na4v7z+oDylcVQ1o2od7 =kuEc -----END PGP SIGNATURE----- |
From: Jorg S. <Jor...@gm...> - 2006-06-01 13:25:36
|
Christophe Fergeau wrote: > Le jeudi 01 juin 2006 à 10:22 +0200, Christophe Fergeau a écrit : > >> Maybe we could have something like that instead: >> >> struct _Itdb_DB >> { >> DBType db_type; >> Itdb_Device *device;/* iPod device info */ >> /* below is for use by application */ >> guint64 usertype; >> gpointer userdata; >> /* function called to duplicate userdata */ >> ItdbUserDataDuplicateFunc userdata_duplicate; >> /* function called to free userdata */ >> ItdbUserDataDestroyFunc userdata_destroy; >> union { >> Itdb_PhotoDB *photodb; >> Itdb_iTunesDB *itdb; >> } >> } >> >> struct _Itdb_PhotoDB { > GList *photos; > GList *photoalbums; >> }; >> >> struct _Itdb_iTunesDB { > GList *tracks; > GList *playlists; >> gchar *filename; /* filename of iTunesDB */ >> guint32 version; >> guint64 id; >> }; Well, Itdb_DB is only used libgpod internally, usertype and userdata are application-specific and would have to go into PhotoDB or iTunesDB anyhow. And Device is associated with the application's iTunesDB or PhotoDB and has to go there as well. That leaves Itdb_DB just the way it is already. A function already exists to access Device in a Itdb_DB: db_get_device(). I'll audit the rest of the Itdb_DB accesses and put in some assertions where there's a possibility for mess-up, so at least we get a clear hint as to what is going wrong if it happens again. Cheers, JCS. |
From: Christophe F. <te...@gn...> - 2006-06-01 13:38:02
|
Le jeudi 01 juin 2006 =C3=A0 22:25 +0900, Jorg Schuler a =C3=A9crit : > Well, Itdb_DB is only used libgpod internally,=20 Ah, ok, I wrote the mail without looking at how these things were actually used ;) We could at least reorder the fields in the PhotoDB and iTunesDB to make sure the common ones are at the same offset.=20 Since Itdb_DB is internal, is it possible not to put its definition in itdb.h which gets installed? > I'll audit the rest of the Itdb_DB accesses and put in some assertions=20 > where there's a possibility for mess-up, so at least we get a clear hin= t=20 > as to what is going wrong if it happens again. Dunno how things are working at the moment, but never accessing db.photodb and db.itunesdb directly, but going through db_get_photodb and db_get_itunesdb accessors which checks the DB type before using the union would avoid bugs such as this one as far as I can tell (please forgive me if I'm totally wrong, I haven't looked at the code much, I'm mainly guessing how things are done ;) Christophe |