Rockbox mail archiveSubject: Re: dreamlayers: r31089 - trunk/apps
Re: dreamlayers: r31089 - trunk/apps
From: Boris Gjenero <boris.gjenero_at_gmail.com>
Date: Sun, 04 Dec 2011 11:25:14 -0500
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
>> --- 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)
>> _at__at_ -98,16 +98,6 _at__at_
>> return filesize(fd);
>> -static int app_closedir(DIR *dirp)
>> - return closedir(dirp);
>> -static struct dirent *app_readdir(DIR *dirp)
>> - return readdir(dirp);
>> #if defined(HAVE_PLUGIN_CHECK_OPEN_CLOSE)&& (MAX_OPEN_FILES>32)
>> _at__at_ -385,11 +375,11 _at__at_
>> /* 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,
> 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
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
> # define rmdir rmdir_cached
> # define rmdir rmdir_uncached
(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
> #if defined(APPLICATION)
> #if (CONFIG_PLATFORM & PLATFORM_ANDROID)
> #include "dir-target.h"
> # 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);
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.
-- BorisReceived on 2011-12-04