On 29/11/2011 3:54 AM, Thomas Martitz wrote:
Date: 2011-11-29 05:56:42 +0100 (Tue, 29 Nov 2011)
New Revision: 31089
Log Message:
FS#12414 : Fix directory functions in plugins on targets which
HAVE_DIRCACHE. In rockbox_api, PREFIX( ) is removed around directory
functions because that's now handled in directory header files. Thanks
to Fred Bauer for reporting this.
Modified:
trunk/apps/plugin.c
Modified: trunk/apps/plugin.c
===================================================================
--- trunk/apps/plugin.c 2011-11-29 00:42:27 UTC (rev 31088)
+++ trunk/apps/plugin.c 2011-11-29 04:56:42 UTC (rev 31089)
@@ -98,16 +98,6 @@
{
return filesize(fd);
}
-
-static int app_closedir(DIR *dirp)
-{
- return closedir(dirp);
-}
-
-static struct dirent *app_readdir(DIR *dirp)
-{
- return readdir(dirp);
-}
#endif
#if defined(HAVE_PLUGIN_CHECK_OPEN_CLOSE)&& (MAX_OPEN_FILES>32)
@@ -385,11 +375,11 @@
filetype_get_attr,
/* dir */
- (opendir_func)PREFIX(opendir),
- (closedir_func)PREFIX(closedir),
- (readdir_func)PREFIX(readdir),
- PREFIX(mkdir),
- PREFIX(rmdir),
+ (opendir_func)opendir,
+ (closedir_func)closedir,
+ (readdir_func)readdir,
+ mkdir,
+ rmdir,
dir_exists,
dir_get_info,
Okay, I admit this fix seemed obvious. But this bug is actually known
for longer (see FS#12032) and I fear it's not that easy.
When I looked into it I found that the above is the culprit indeed.
However, one needs to look into svn history (revision r28929, and more
importantly r28927) to see that the PREFIX() have been introduced for a
reason (to fix FS#11844).
That said, I haven't looked any further, so I don't know what the
correct fix is or if the above is incorrect. But I wonder if the reason
it was introduced was considered, as it bascally reverts most of r28929.
I'm sorry if I missed something obvious. I'm not asking for reversal but
for clarification.
Best regards.
Take a look at firmware/include/dir_uncached.h. There you see:
#if (CONFIG_PLATFORM & (PLATFORM_SDL|PLATFORM_MAEMO|PLATFORM_PANDORA))
# define dirent_uncached sim_dirent
# define DIR_UNCACHED SIM_DIR
# define opendir_uncached sim_opendir
# define readdir_uncached sim_readdir
# define closedir_uncached sim_closedir
# define mkdir_uncached sim_mkdir
# define rmdir_uncached sim_rmdir
#endif
In firmware/export/config/sim.h, you have "#define CONFIG_PLATFORM
(PLATFORM_HOSTED|PLATFORM_SDL)" As a result, sim prefixes are applied
for the uncached directory types and functions.
In firmware/include/dir.h, you have:
#ifdef HAVE_DIRCACHE
*snip*
# define rmdir rmdir_cached
#else
*snip*
# define rmdir rmdir_uncached
#endif
(It also handles the other stuff; I just edited here it to shorten it.)
On the sim, if HAVE_DIRCACHE is defined, plugins access directories via
dircache, and dircache accesses them via the sim_ functions. On sims
without dircache, directory access is via the _uncached functions, which
are actually the sim_ functions.
Applications have their own section in below the sim_ section in
dir_uncached.h
#if defined(APPLICATION)
#if (CONFIG_PLATFORM & PLATFORM_ANDROID)
#include "dir-target.h"
#endif
# undef opendir_uncached
# define opendir_uncached app_opendir
# undef mkdir_uncached
# define mkdir_uncached app_mkdir
# undef rmdir_uncached
# define rmdir_uncached app_rmdir
/* defined in rbpaths.c */
extern DIR_UNCACHED* app_opendir(const char* name);
extern int app_rmdir(const char* name);
extern int app_mkdir(const char* name);
#endif
In this case, you see the same thing happens for all functions other
than readdir() and closedir(). If you scroll back to the top, you'll see
that the removed app_readdir() and app_closedir() functions in plugin.c
only called readdir() and closedir(). The other app_ functions exist
because paths need to be translated. The other _uncached functions may
be sim_ functions from earlier defines.
I didn't remove the opendir_func, closedir_func and readdir_func
typedefs and casts because they weren't causing problems, I didn't
understand their purpose, and I wasn't sure that their removal wouldn't
cause compile errors in some situations. If they're unneeded, they
should be removed.
When creating the r31089 patch, I only examined current code. It would
have been better if I looked at history, found r28929, and contacted you
about reverting that. Sorry about that.
File functions have the same system, via firmware/include/file.h.
However, the defines for directory functions simply define names, and
defines for the file functions use function-like macros such as:
# define creat(x,m) sim_creat(x,m)
In this case, in a sim, a call to creat() will call sim_creat(), but
creat means the address of the system creat() function. So, PREFIX() is
required on file functions, and r28927 was needed to fix a problem.
However, I wonder if it would be better to remove the parameters from
the defines, allowing things to work without PREFIX().
I hope this helps explain r31089 changes.
--
Boris
