On Tue, 5 Jan 2010, Max Kellermann wrote: > On 2010/01/05 22:50, Jeff Frasca <phaed...@u.washington.edu> wrote: >>> On Tue, 5 Jan 2010, Max Kellermann wrote: >>>> The tab command should be disabled with --enable-mini. It's unlikely >>>> to be useful on those tiny platforms. >> >> Alright, I've got a mockup of the 'mpc radd' code working. It's >> available at: >> >> git pull http://sasquatch-labs.org/git/mpc.git mpc-add-regex >> >> (That repo has everything I've done merged in master, so pulling master >> from there should have the same effect.) > > - don't include regex.h if !MPC_SMALL_BUILD
*facepalm* Yep, that should be #ifndef'd against MPC_SMALL_BUILD. > - turn the loop around: first parse all regular expressions, then > iterate through the list of songs, applying all regular expressions > to that URI until one matches; eliminates the manual duplicate > detection Yeah, that's a better idea, and if I want the adds done in sort order, I'll need to do that anyway. > - why memset(fnames)? Paranoia. I noted that with two XXX: comments in the vicinity (strictly, either the explicit sentinal set at the end of the loop or the memset is necessary, and the memset is more expensive). I was planning on pulling it. > - why use a temporary file? You could either allocate all song URIs > on the heap (may be expensive, but I don't think that's a problem) > or open a second MPD connection for adding while receiving songs on > the first one > > - why are you adding _all_ songs to the temporary file, and apply > regex later? You could save a lot of space by doing the filter > first For both of these, I'm making the kernel do my memory management for me. The point of all the songs in the temp file is that I can sort them (I haven't added that yet), which I really want it to do so that the adds come out in sort order--which will add my albums in track order instead of interleaved (I use one directory per artist to manage my music files). > - you can close(tmpf) right after mmap(), the file descriptor isn't > needed after the mmap has been established Good point. I didn't realize that was ok. > - do we need a configure.ac test for regex.h? Is that portable? > (what about mingw32?) > > - is mkstemp() portable to mingw32? > > - is mmap() portable to mingw32? mmap() and regex are part of POSIX. mmap() is not portable to mingw32. Other than that, I have no idea. I have never worked with mingw32. I can add a check for regex.h into the configure script and turn on MPC_SMALL_BUILD if it doesn't work. > It is particularly important that we don't break the mingw32 build. > Can you test that, please? (--host=i586-mingw32msvc) I single boot Slackware Linux. So, no, I cannot. I'll add tests into the build system to disable radd in mingw32. Jeff ------------------------------------------------------------------------------ This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev _______________________________________________ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
