[vlc-devel] [PATCH] Basic MTP Support
funman at videolan.org
Wed Mar 26 23:45:39 CET 2008
2008/3/26, Fabio Ritrovato <exsephiroth87 at gmail.com>:
> Well, funman said that editing configure.ac was not necessarily for my
but it is better .. ;)
At a start you can just copy the detection from another module, and
change it to your needs.
Open Source is great because you can copy/paste (but don't paste bugs !)
Now about the patch:
+ * Authors: Fabio Ritrovato <exsephiroth87 at gmail.com>
+ * Christophe Massiot <massiot at via.ecp.fr>
+ * Rémi Denis-Courmont <rem # videolan # org>
Did they help you to code ?
You don't use that one
+ char to_path;
+ strcat( to_path, getenv( "HOME" ) );
+ strcat( to_path, "/.vlc/" );
+ strcat( to_path, p_access->psz_path );
+ msg_Dbg( p_access, "About to write %s...", to_path );
There is an obvious buffer overflow.
I'm not sure to understand why you have to write something in ~/.vlc ?
+ current = malloc (strlen(parent) + strlen(folder->name) + 2);
You don't check if malloc() succeeded or not
Are you sure the access module has not unneeded parts from file.c ?
I'll jump to services discovery:
+ * Copyright (c) 2006-2007 Rafaël Carré
+# include <unistd.h>
Leftover debug code ?
+ services_discovery_sys_t *p_sys = malloc(
+ sizeof( services_discovery_sys_t ) );
You don't check if malloc() failed
+ psz_name = LIBMTP_Get_Modelname( p_device );
+ msg_Info( p_sd, "%s", psz_name );
Is that function guaranteed to succeed ?
If not you should use a fallback like _("Unknown")
+ services_discovery_SetLocalizedName( p_sd, _(psz_name) );
_() returns the translated version of a string, however you can not
use it on an "user-provided" (in this case library provided) string,
since it is not known to translators.
Here it should be something like _("MTP devices")
+ msg_Info( p_sd, "No device found..." );
+ free( p_sys );
Too bad :( can't you detect devices addition ? For example I launch
VLC, and THEN I connect my mtp device
The same apply for Run() function
static void Close( vlc_object_t *p_this )
+ services_discovery_t *p_sd = ( services_discovery_t* )p_this;
+ free( p_sd->p_sys );
It seems strange to me, you don't have to clean anything allocated by libmtp ?
+ sprintf(psz_path, "mtp://%s", p_track->filename);
malloc() not checked
It seems the libmtp plays file mounted on a filesystem, I thought it
was not needed, was I wrong ?
Thanks for your work, I hope to see an updated patch soon ;)
More information about the vlc-devel