Re: adding directory contents to ogg123 playing in sorted order?

On Sun, Feb 28, 2010 at 11:11 PM, Ray Kohler <[email protected]> wrote:
> On Sun, Feb 28, 2010 at 11:01 PM,  <[email protected]> wrote:
>>>> Yes, we'd take such a patch.  _Especially_ if you do the extra build
>>>> system legwork to establish portability. :-)  Now is good timing too,
>>>> I intend to release a new ogg123 this month.
>>>
>>> Ok, I will work on this soon, hopefully tomorrow. I have to learn
>>> autotools practically from scratch, so this could give me a little
>>> trouble.
>>
>> Then don't worry about that part.  If you could look to see where
>> support does and doesn't exist, that still helps.
>
> The new functions I'd be using are scandir(3) and alphasort(3). Both
> are BSD-derived. I know they are also present on Solaris 8+ and HP-UX
> 11i v2, as well as any modern glibc, as that's what I have direct
> access to. It's likely that they're available on any SysV-based system
> that ships a BSD-compatibility layer as well. That's quite possibly
> everything.
>
> I'll make a first pass that basically replaces
> playlist_append_directory() entirely, as I'm guessing that's probably
> the easiest way to handle two different cases at compile-time.

This turned out to be very easy - I can't remember the last time I
wrote some code and had it work the first time. Since the relevant
functions are in libc, the autoconf changes were very easy as well, so
I did those too. Given that this is a short function, and changes are
throughout, I judged it more readable to have two complete copies of
the function, rather than one function with lots of #if defined blocks
- the preprocessor directives would practically outnumber the code if
I had done it that way. Patch follows, and is against current SVN
trunk. (I'm also adding it as an attachment, as I don't trust mail not
to mangle it.)

Index: configure.ac
===================================================================
--- configure.ac	(revision 16870)
+++ configure.ac	(working copy)
@@ -317,7 +317,7 @@

 AC_FUNC_ALLOCA
 AM_ICONV
-AC_CHECK_FUNCS(atexit on_exit fcntl select stat chmod)
+AC_CHECK_FUNCS(atexit on_exit fcntl select stat chmod alphasort scandir)
 AM_LANGINFO_CODESET

 dnl --------------------------------------------------
Index: ogg123/playlist.c
===================================================================
--- ogg123/playlist.c	(revision 16870)
+++ ogg123/playlist.c	(working copy)
@@ -120,8 +120,63 @@
 }

 /* Recursively adds files from the directory and subdirectories */
+#if defined(HAVE_ALPHASORT) && defined(HAVE_SCANDIR)
 int playlist_append_directory(playlist_t *list, char *dirname)
 {
+  int dir_len = strlen(dirname);
+  int num_entries = 0, i = 0;
+  struct dirent **entries;
+  struct stat stat_buf;
+  char nextfile[NAME_MAX + 1];
+
+  num_entries = scandir(dirname, &entries, 0, alphasort);
+
+  if (num_entries < 0) {
+    return 0;
+  }
+
+  for (i=0; i<num_entries; i++) {
+    int sub_len = strlen(entries[i]->d_name);
+
+    /* Make sure full pathname is within limits and we don't parse the
+       relative directory entries. */
+    if (dir_len + sub_len + 1 < NAME_MAX
+        && strcmp(entries[i]->d_name, ".") != 0
+        && strcmp(entries[i]->d_name, "..") != 0  ) {
+
+      /* Build the new full pathname */
+      strcpy(nextfile, dirname);
+      strcat(nextfile, "/");
+      strcat(nextfile, entries[i]->d_name);
+
+      if (stat(nextfile, &stat_buf) == 0) {
+
+        /* Decide what type of entry this is */
+        if (S_ISDIR(stat_buf.st_mode)) {
+
+        /* Recursively follow directories */
+          if ( playlist_append_directory(list, nextfile) == 0 ) {
+            fprintf(stderr,
+                  _("Warning: Could not read directory %s.\n"),
+                    nextfile);
+          }
+        } else {
+        /* Assume everything else is a file of some sort */
+          playlist_append_file(list, nextfile);
+        }
+      }
+    }
+
+    free(entries[i]);
+  }
+
+  free(entries);
+
+  return 1;
+}
+#else
+int playlist_append_directory(playlist_t *list, char *dirname)
+{
   DIR *dir;
   int dir_len = strlen(dirname);
   struct dirent *entry;
@@ -174,6 +229,7 @@

   return 1;
 }
+#endif


 /* Opens a file containing filenames, one per line, and adds them to the

_______________________________________________
Vorbis-dev mailing list
[email protected]
http://lists.xiph.org/mailman/listinfo/vorbis-dev

Index: configure.ac
===================================================================
--- configure.ac	(revision 16870)
+++ configure.ac	(working copy)
@@ -317,7 +317,7 @@
 
 AC_FUNC_ALLOCA
 AM_ICONV
-AC_CHECK_FUNCS(atexit on_exit fcntl select stat chmod)
+AC_CHECK_FUNCS(atexit on_exit fcntl select stat chmod alphasort scandir)
 AM_LANGINFO_CODESET
 
 dnl --------------------------------------------------
Index: ogg123/playlist.c
===================================================================
--- ogg123/playlist.c	(revision 16870)
+++ ogg123/playlist.c	(working copy)
@@ -120,8 +120,63 @@
 }
 
 /* Recursively adds files from the directory and subdirectories */
+#if defined(HAVE_ALPHASORT) && defined(HAVE_SCANDIR)
 int playlist_append_directory(playlist_t *list, char *dirname)
 {
+  int dir_len = strlen(dirname);
+  int num_entries = 0, i = 0;
+  struct dirent **entries;
+  struct stat stat_buf;
+  char nextfile[NAME_MAX + 1];
+
+  num_entries = scandir(dirname, &entries, 0, alphasort);
+
+  if (num_entries < 0) {
+    return 0;
+  }
+
+  for (i=0; i<num_entries; i++) {
+    int sub_len = strlen(entries[i]->d_name);
+
+    /* Make sure full pathname is within limits and we don't parse the
+       relative directory entries. */
+    if (dir_len + sub_len + 1 < NAME_MAX 
+        && strcmp(entries[i]->d_name, ".") != 0  
+        && strcmp(entries[i]->d_name, "..") != 0  ) {
+
+      /* Build the new full pathname */
+      strcpy(nextfile, dirname);
+      strcat(nextfile, "/");
+      strcat(nextfile, entries[i]->d_name);
+
+      if (stat(nextfile, &stat_buf) == 0) {
+        
+        /* Decide what type of entry this is */
+        if (S_ISDIR(stat_buf.st_mode)) {
+          
+        /* Recursively follow directories */
+          if ( playlist_append_directory(list, nextfile) == 0 ) {
+            fprintf(stderr,
+                  _("Warning: Could not read directory %s.\n"), 
+                    nextfile);
+          }
+        } else {
+        /* Assume everything else is a file of some sort */
+          playlist_append_file(list, nextfile);
+        }
+      }
+    }
+
+    free(entries[i]);
+  }
+
+  free(entries);
+
+  return 1;
+}
+#else
+int playlist_append_directory(playlist_t *list, char *dirname)
+{
   DIR *dir;
   int dir_len = strlen(dirname);
   struct dirent *entry;
@@ -174,6 +229,7 @@
 
   return 1;
 }
+#endif
 
 
 /* Opens a file containing filenames, one per line, and adds them to the