Hi EV, On Tue, Aug 15, 2006, EV wrote: > The attached patch is against changeset 97, which is tobe > obtained by aplying Keith's tags-libkarma-hg96.patch to the > current repository tip, changeset 96. > > Apart from the error handling, I've fixed some rather minor > issues mostly related with missing properties/tags.
I have a couple of queries regarding this patch. > + > +#ifndef TagLib_File_FLAC /* older tag_c libs don't have TagLib_File_FLAC */ > +#define TagLib_File_FLAC -1 > +#endif Was this because it had no support for FLAC files? If that's the case then it would make more sense to return an error rather than silently carry on. > + if(type != -1) > + tl_file = taglib_file_new_type(filename, type); > + else > + tl_file = taglib_file_new(filename); > tl_file = taglib_file_new_type(filename, type); > tl_tag = taglib_file_tag(tl_file); This is in error due to the second taglib_file_new_type() call. Asides from that, I think that this is not guaranteed to work. taglib seems to be a bit dumb when it comes to autodetecting file types and when it fails it will set tl_file to NULL. The taglib_file_tag() will then segfault. If TagLib_File_FLAC is undefined because there is no support for FLAC files then this is a pointless test which is guaranteed to fail anyway. On the other hand, if older versions of taglib do support FLAC files but just don't provide a file type flag, then an extra test is needed to check that the taglib_file_new() has succeeded. Keith. ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ linux-karma-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-karma-devel
