[PATCH] UDF+ISO image extraction sample (and a plan for upcoming changes)

[Pete Batard]

Alright, now that I have straightened out a working UDF+ISO extraction
in my application (or at least what looks like one so far), I am in a
position to try to feed back a few fixes and improvements to libcdio.

Naturally, I don't expect all the modifications I have currently applied
to the source to work in my app, to be transposable as is, especially as
I took some liberties here and there. But at the very leats, I should be
able to provide details on how I addressed the specific issues that I
had, and we can try to take it from there.

Overall, even though I almost gave up on libcdio in despair a few days
ago, not much seems to be needed to get UDF support going.

For people interested in testing the upcoming changes, the first thing
we are going to need is a new sample that does image extraction, which
is provided in the attached patch. It's heavily based on what the
existing iso and udf samples already do for directory listing and file
extraction, the main different being that it avoid using ftruncate
because ultimately we would need an ftruncate64 (we'll be handling some
files that are > 4GB in size) and this becomes a cross-platform
headache, especially if one plans to eventually support MSVC compilers.
The current sample does not do timestamp/premissions preservation and
does not deal with Unicode (unless the underlying calls support UTF-8,
which wouldn't be the case on Windows), as this isn't really something
we should have issues with. One I have sorted these in my app, I'll
probably update the sample though.

Before I go ahead an apply it to mainline, I'd appreciate if you could
review the sample to confirm that it looks OK. Unless I get a green
light, I'm not planning to push anything to mainline for another day or two.

The second item we will need besides that sample, is an UDF image that
can exhibit issues that need to be addressed. The one I would encourage
everyone to use would be the "Windows 8 Developer Preview with developer
tools English" since:
1. It is freely available from Microsoft
2. It is more than 4 GB in size (=> will test 32 bit limitations)
3. It also contains a file that is more than 4 GB in size (=> more 32
bit test, as well as extended attributes use)
This image can be downloaded at: [1] (direct link [2]).

With the sample and the image file above, we should then be able to:

1. Make UDF extraction work on Linux 32 bit (note that 64 bit Linux may
not exhibit the 32 bit problems I've seen, so I'd strongly encourage to
use 32 bit Linux for tests)

2. Make UDF extraction work on MinGW32 as well as other platforms that
don't have _FILE_OFFSET_BITS 64 or transparent large file support

3. Make UDF and ISO9660 image extraction work with MSVC on Windows


As you may guess, my objective is to bridge as much as the gap as
possible between the libcdio source I use in my app (which is compiled
with both MinGW and MSVC), and official. Eventually, I'd like to be able
to use mainline files as is, to easily benefit from future libcdio
fixes/updates.

Finally, because I am only interested in image extraction at the moment,
and it should be enough to keep us busy for some time (especially if we
add MSVC support), I'm going to be very restrictive in the scope of
libcdio for this whole exercise. In effect, the following options are
what I'll use on all platforms (which you may want to add to your
autogen.sh for testing)

./configure --enable-maintainer-mode --disable-cddb \
   --disable-vcd-info --disable-cxx --disable-cpp-progs \
   --without-cd-drive --without-cd-info --without-cd-paranoia \
   --without-cdda-player --without-cd-read

With this sorted out, we are ready to start patching libcdio...

Regards,

/Pete

[1] http://msdn.microsoft.com/en-us/windows/apps/br229516
[2]
http://wdp.dlws.microsoft.com/WDPDL/9B8DFDFF736C5B1DBF956B89D8A9D4FD925DACD2/WindowsDeveloperPreview-64bit-English-Developer.iso

From 4616028005dfedc6580baa75d5390a9696921ffc Mon Sep 17 00:00:00 2001
From: Pete Batard <pete@...>
Date: Sun, 22 Jan 2012 02:00:37 +0000
Subject: [PATCH] Add ISO9660+UDF image extraction example

---
 example/Makefile.am |    5 +-
 example/extract.c   |  270 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 274 insertions(+), 1 deletions(-)
 create mode 100644 example/extract.c

diff --git a/example/Makefile.am b/example/Makefile.am
index de65b5b..995f619 100644
--- a/example/Makefile.am
+++ b/example/Makefile.am
@@ -26,7 +26,7 @@ paranoia_progs = paranoia paranoia2
 endif
 if BUILD_EXAMPLES
 noinst_PROGRAMS = audio cdchange cdtext device discid drives eject \
-          isofile isofile2 isofuzzy isolist isolsn \
+          extract isofile isofile2 isofuzzy isolist isolsn \
           mmc1 mmc2 mmc2a mmc3 $(paranoia_progs) tracks \
           sample3 sample4 udf1 udffile cdio-eject
 endif
@@ -54,6 +54,9 @@ drives_LDADD          = $(LIBCDIO_LIBS) $(LTLIBICONV)
 eject_DEPENDENCIES    = $(LIBCDIO_DEPS)
 eject_LDADD           = $(LIBCDIO_LIBS) $(LTLIBICONV)
 
+extract_DEPENDENCIES  = $(LIBISO9660_LIBS) $(LIBUDF_LIBS) $(LIBCDIO_DEPS)
+extract_LDADD         = $(LIBISO9660_LIBS) $(LIBUDF_LIBS) $(LIBCDIO_LIBS) $(LTLIBICONV)
+
 cdio_eject_DEPENDENCIES = $(LIBCDIO_DEPS)
 cdio_eject_LDADD        = $(LIBCDIO_LIBS) $(LTLIBICONV)
 
diff --git a/example/extract.c b/example/extract.c
new file mode 100644
index 0000000..30644db
--- /dev/null
+++ b/example/extract.c
@@ -0,0 +1,270 @@
+/*
+  Copyright (C) 2012 Pete Batard <pete@...>
+  Based on samples copyright (c) 2003-2011 Rocky Bernstein <rocky@...>
+  
+  This program is free software: you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation, either version 3 of the License, or
+  (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/* Extract the full content of either an UDF or ISO9660
+   TODO: timestamp preservation, file permissions, Unicode
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <malloc.h>
+#include <errno.h>
+
+#include <cdio/cdio.h>
+#include <cdio/logging.h>
+#include <cdio/iso9660.h>
+#include <cdio/udf.h>
+
+#if defined(_WIN32)
+#include <direct.h>
+#else
+#include <sys/stat.h>
+#include <sys/types.h>
+#define _mkdir(a) mkdir(a, S_IRWXU)
+#endif
+
+#ifndef MIN
+#define MIN(a,b) (((a) < (b)) ? (a) : (b))
+#endif
+
+#define print_vd_info(title, fn)     \
+  if (fn(p_iso, &psz_str)) {         \
+    printf(title ": %s\n", psz_str); \
+  }                                  \
+  free(psz_str);                     \
+  psz_str = NULL;
+
+const char *psz_extract_dir;
+
+static int udf_extract_files(udf_t *p_udf, udf_dirent_t *p_udf_dirent, const char *psz_path)
+{
+  FILE *fd = NULL;
+  int i_length;
+  char* psz_fullpath;
+  const char* psz_basename;
+  udf_dirent_t *p_udf_dirent2;
+  uint8_t buf[UDF_BLOCKSIZE];
+  int64_t i_read, i_file_length;
+
+  if ((p_udf_dirent == NULL) || (psz_path == NULL))
+    return 1;
+
+  while (udf_readdir(p_udf_dirent)) {
+    psz_basename = udf_get_filename(p_udf_dirent);
+    i_length = 3 + strlen(psz_path) + strlen(psz_basename) + strlen(psz_extract_dir);
+    psz_fullpath = (char*)calloc(sizeof(char), i_length);
+    if (psz_fullpath == NULL) {
+      fprintf(stderr, "Error allocating file name\n");
+      goto out;
+    }
+    i_length = snprintf(psz_fullpath, i_length, "%s%s/%s", psz_extract_dir, psz_path, psz_basename);
+    if (i_length < 0) {
+      goto out;
+    }
+    printf("Extracting: %s\n", psz_fullpath);
+    if (udf_is_dir(p_udf_dirent)) {
+      _mkdir(psz_fullpath);
+      p_udf_dirent2 = udf_opendir(p_udf_dirent);
+      if (p_udf_dirent2 != NULL) {
+        if (udf_extract_files(p_udf, p_udf_dirent2, &psz_fullpath[strlen(psz_extract_dir)]))
+          goto out;
+      }
+    } else {
+      fd = fopen(psz_fullpath, "wb");
+      if (fd == NULL) {
+        fprintf(stderr, "  Unable to create file\n");
+        goto out;
+      }
+      i_file_length = udf_get_file_length(p_udf_dirent);
+      while (i_file_length > 0) {
+        memset(buf, 0, UDF_BLOCKSIZE);
+        i_read = udf_read_block(p_udf_dirent, buf, 1);
+        if (i_read < 0) {
+          fprintf(stderr, "  Error reading UDF file %s\n", &psz_fullpath[strlen(psz_extract_dir)]);
+          goto out;
+        }
+        fwrite(buf, (size_t)MIN(i_file_length, i_read), 1, fd);
+        if (ferror(fd)) {
+          fprintf(stderr, "  Error writing file\n");
+          goto out;
+        }
+        i_file_length -= i_read;
+      }
+      fclose(fd);
+      fd = NULL;
+    }
+    free(psz_fullpath);
+  }
+  return 0;
+
+out:
+  if (fd != NULL)
+    fclose(fd);
+  free(psz_fullpath);
+  return 1;
+}
+
+static int iso_extract_files(iso9660_t* p_iso, const char *psz_path)
+{
+  FILE *fd = NULL;
+  int i_length, r = 1;
+  char psz_fullpath[4096], *psz_basename;
+  const char *psz_iso_name = &psz_fullpath[strlen(psz_extract_dir)];
+  unsigned char buf[ISO_BLOCKSIZE];
+  CdioListNode_t* p_entnode;
+  iso9660_stat_t *p_statbuf;
+  CdioList_t* p_entlist;
+  size_t i;
+  lsn_t lsn;
+  int64_t i_file_length;
+
+  if ((p_iso == NULL) || (psz_path == NULL))
+    return 1;
+
+  i_length = snprintf(psz_fullpath, sizeof(psz_fullpath), "%s%s/", psz_extract_dir, psz_path);
+  if (i_length < 0)
+    return 1;
+  psz_basename = &psz_fullpath[i_length];
+
+  p_entlist = iso9660_ifs_readdir(p_iso, psz_path);
+  if (!p_entlist)
+    return 1;
+
+  _CDIO_LIST_FOREACH (p_entnode, p_entlist) {
+    p_statbuf = (iso9660_stat_t*) _cdio_list_node_data(p_entnode);
+    /* Eliminate . and .. entries */
+    if ( (strcmp(p_statbuf->filename, ".") == 0)
+      || (strcmp(p_statbuf->filename, "..") == 0) )
+      continue;
+    iso9660_name_translate(p_statbuf->filename, psz_basename);
+    if (p_statbuf->type == _STAT_DIR) {
+      _mkdir(psz_fullpath);
+      if (iso_extract_files(p_iso, psz_iso_name))
+        goto out;
+    } else {
+      printf("Extracting: %s\n", psz_fullpath);
+      fd = fopen(psz_fullpath, "wb");
+      if (fd == NULL) {
+        fprintf(stderr, "  Unable to create file\n");
+        goto out;
+      }
+      i_file_length = p_statbuf->size;
+      for (i = 0; i_file_length > 0; i++) {
+        memset(buf, 0, ISO_BLOCKSIZE);
+        lsn = p_statbuf->lsn + i;
+        if (iso9660_iso_seek_read(p_iso, buf, lsn, 1) != ISO_BLOCKSIZE) {
+          fprintf(stderr, "  Error reading ISO9660 file %s at LSN %lu\n",
+            psz_iso_name, (long unsigned int)lsn);
+          goto out;
+        }
+        fwrite(buf, (size_t)MIN(i_file_length, ISO_BLOCKSIZE), 1, fd);
+        if (ferror(fd)) {
+          fprintf(stderr, "  Error writing file\n");
+          goto out;
+        }
+        i_file_length -= ISO_BLOCKSIZE;
+      }
+      fclose(fd);
+      fd = NULL;
+    }
+  }
+  r = 0;
+
+out:
+  if (fd != NULL)
+    fclose(fd);
+  _cdio_list_free(p_entlist, true);
+  return r;
+}
+
+int main(int argc, char** argv)
+{
+  iso9660_t* p_iso = NULL;
+  udf_t* p_udf = NULL;
+  udf_dirent_t* p_udf_root;
+  char *psz_str = NULL;
+  char vol_id[UDF_VOLID_SIZE] = "";
+  char volset_id[UDF_VOLSET_ID_SIZE+1] = "";
+  int r = 0;
+
+  cdio_loglevel_default = CDIO_LOG_DEBUG;
+  
+  if (argc < 3) {
+    fprintf(stderr, "Usage: extract <iso_image> <extraction_dir>\n");
+    return 1;
+  }
+
+  psz_extract_dir = argv[2];
+  if (_mkdir(psz_extract_dir) == 0) {
+    printf("Creating directory: %s\n", psz_extract_dir);
+  } else if (errno != EEXIST) {
+    fprintf(stderr, "Unable to create extraction directory %s\n", psz_extract_dir);
+    return 1;
+  }
+
+  /* First try to open as UDF - fallback to ISO if it failed */
+  p_udf = udf_open(argv[1]);
+  if (p_udf == NULL)
+    goto try_iso;
+
+  p_udf_root = udf_get_root(p_udf, true, 0);
+  if (p_udf_root == NULL) {
+    fprintf(stderr, "Couldn't locate UDF root directory\n");
+    goto out;
+  }
+  vol_id[0] = 0; volset_id[0] = 0;
+  
+  /* Show basic UDF Volume info */
+  if (udf_get_volume_id(p_udf, vol_id, sizeof(vol_id)) > 0)
+    fprintf(stderr, "Volume id: %s\n", vol_id);
+  if (udf_get_volume_id(p_udf, volset_id, sizeof(volset_id)) >0 ) {
+    volset_id[UDF_VOLSET_ID_SIZE]='\0';
+    fprintf(stderr, "Volume set id: %s\n", volset_id);
+  }
+  fprintf(stderr, "Partition number: %d\n", udf_get_part_number(p_udf));
+
+  /* Recursively extract files */
+  r = udf_extract_files(p_udf, p_udf_root, "");
+
+  goto out;
+
+try_iso:
+  p_iso = iso9660_open(argv[1]);
+  if (p_iso == NULL) {
+    fprintf(stderr, "Unable to open image '%s'.\n", argv[1]);
+    goto out;
+  }
+
+  /* Show basic ISO9660 info from the Primary Volume Descriptor. */
+  print_vd_info("Application", iso9660_ifs_get_application_id);
+  print_vd_info("Preparer   ", iso9660_ifs_get_preparer_id);
+  print_vd_info("Publisher  ", iso9660_ifs_get_publisher_id);
+  print_vd_info("System     ", iso9660_ifs_get_system_id);
+  print_vd_info("Volume     ", iso9660_ifs_get_volume_id);
+  print_vd_info("Volume Set ", iso9660_ifs_get_volumeset_id);
+
+  r = iso_extract_files(p_iso, "");
+
+out:
+  if (p_iso != NULL)
+    iso9660_close(p_iso);
+  if (p_udf != NULL)
+    udf_close(p_udf);
+
+  return r;
+}
--
1.7.8.msysgit.0

[r-]

On Sun, Jan 22, 2012 at 7:01 PM, Pete Batard <pbatard@...> wrote:

> Alright, now that I have straightened out a working UDF+ISO extraction in
> my application (or at least what looks like one so far), I am in a position
> to try to feed back a few fixes and improvements to libcdio.
>

Great!


>
> Naturally, I don't expect all the modifications I have currently applied
> to the source to work in my app, to be transposable as is, especially as I
> took some liberties here and there. But at the very leats, I should be able
> to provide details on how I addressed the specific issues that I had, and
> we can try to take it from there.
>

If it improves your usage it will probably improve other's usage. From what
I've seen so far from the changes I don't see way things are going to get
worse for people.


>
> Overall, even though I almost gave up on libcdio in despair a few days
> ago, not much seems to be needed to get UDF support going.
>

Good to hear.

>
> For people interested in testing the upcoming changes, the first thing we
> are going to need is a new sample that does image extraction, which is
> provided in the attached patch. It's heavily based on what the existing iso
> and udf samples already do for directory listing and file extraction, the
> main different being that it avoid using ftruncate because ultimately we
> would need an ftruncate64 (we'll be handling some files that are > 4GB in
> size) and this becomes a cross-platform headache, especially if one plans
> to eventually support MSVC compilers.
> The current sample does not do timestamp/premissions preservation and does
> not deal with Unicode (unless the underlying calls support UTF-8, which
> wouldn't be the case on Windows), as this isn't really something we should
> have issues with. One I have sorted these in my app, I'll probably update
> the sample though.
>

Sample programs don't have to be perfect and handle all cases. On the other
hand, if there is something that is going to have to be coded again and
again, then perhaps the code should be put in the library.



>
> Before I go ahead an apply it to mainline, I'd appreciate if you could
> review the sample to confirm that it looks OK. Unless I get a green light,
> I'm not planning to push anything to mainline for another day or two.
>

To make it easier for others to test things out, I've created a pbatard
branch and committed the patch here and the next one to reset
_udf->i_position.

You can commit future changes to the pbatard branch, and then when folks
feel the time is read this can be merged in the master branch.

>
> The second item we will need besides that sample, is an UDF image that can
> exhibit issues that need to be addressed. The one I would encourage
> everyone to use would be the "Windows 8 Developer Preview with developer
> tools English" since:
> 1. It is freely available from Microsoft
> 2. It is more than 4 GB in size (=> will test 32 bit limitations)
> 3. It also contains a file that is more than 4 GB in size (=> more 32 bit
> test, as well as extended attributes use)
> This image can be downloaded at: [1] (direct link [2]).
>

We can't include this in the distribution. In fact anything that is 4GB in
size can't be included in the distribution.

Conceivably one could add some sort of Makefile target to issue a
retrieval. Then there is the thing that is unsettling to me that this link
could be broken at any time.

Independent of that one wants the ability to test code. I don't think it
reasonable to ask people to download 4GB just to test UDF handling,
although it may be fine to suggest that for more complete or realistic
tests.

Specifically, it is not clear that one couldn't come up with a very small
UDF image that exhibits the _udf->i_position problem of your next patch.

> With the sample and the image file above, we should then be able to:
>
> 1. Make UDF extraction work on Linux 32 bit (note that 64 bit Linux may
> not exhibit the 32 bit problems I've seen, so I'd strongly encourage to use
> 32 bit Linux for tests)
>
> 2. Make UDF extraction work on MinGW32 as well as other platforms that
> don't have _FILE_OFFSET_BITS 64 or transparent large file support
>
> 3. Make UDF and ISO9660 image extraction work with MSVC on Windows
>
>
> As you may guess, my objective is to bridge as much as the gap as possible
> between the libcdio source I use in my app (which is compiled with both
> MinGW and MSVC), and official. Eventually, I'd like to be able to use
> mainline files as is, to easily benefit from future libcdio fixes/updates.
>
> Finally, because I am only interested in image extraction at the moment,
> and it should be enough to keep us busy for some time (especially if we add
> MSVC support), I'm going to be very restrictive in the scope of libcdio for
> this whole exercise.

Perfectly reasonable. I

> In effect, the following options are what I'll use on all platforms (which
> you may want to add to your autogen.sh for testing)
>
> ./configure --enable-maintainer-mode --disable-cddb \
>  --disable-vcd-info --disable-cxx --disable-cpp-progs \
>  --without-cd-drive --without-cd-info --without-cd-paranoia \
>  --without-cdda-player --without-cd-read

> With this sorted out, we are ready to start patching libcdio...
>
> Regards,
>
> /Pete
>
> [1] http://msdn.microsoft.com/en-**us/windows/apps/br229516<http://msdn.microsoft.com/en-us/windows/apps/br229516>
> [2] http://wdp.dlws.microsoft.com/**WDPDL/**9B8DFDFF736C5B1DBF956B89D8A9D4
> **FD925DACD2/**WindowsDeveloperPreview-64bit-**English-Developer.iso<http://wdp.dlws.microsoft.com/WDPDL/9B8DFDFF736C5B1DBF956B89D8A9D4FD925DACD2/WindowsDeveloperPreview-64bit-English-Developer.iso>
>
> From 4616028005dfedc6580baa75d5390a9696921ffc Mon Sep 17 00:00:00 2001
> From: Pete Batard <pete@...>
> Date: Sun, 22 Jan 2012 02:00:37 +0000
> Subject: [PATCH] Add ISO9660+UDF image extraction example
>
> ---
>  example/Makefile.am |    5 +-
>  example/extract.c   |  270
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 274 insertions(+), 1 deletions(-)
>  create mode 100644 example/extract.c
>
> diff --git a/example/Makefile.am b/example/Makefile.am
> index de65b5b..995f619 100644
> --- a/example/Makefile.am
> +++ b/example/Makefile.am
> @@ -26,7 +26,7 @@ paranoia_progs = paranoia paranoia2
>  endif
>  if BUILD_EXAMPLES
>  noinst_PROGRAMS = audio cdchange cdtext device discid drives eject \
> -                 isofile isofile2 isofuzzy isolist isolsn \
> +                 extract isofile isofile2 isofuzzy isolist isolsn \
>                  mmc1 mmc2 mmc2a mmc3 $(paranoia_progs) tracks \
>                  sample3 sample4 udf1 udffile cdio-eject
>  endif
> @@ -54,6 +54,9 @@ drives_LDADD          = $(LIBCDIO_LIBS) $(LTLIBICONV)
>  eject_DEPENDENCIES    = $(LIBCDIO_DEPS)
>  eject_LDADD           = $(LIBCDIO_LIBS) $(LTLIBICONV)
>
> +extract_DEPENDENCIES  = $(LIBISO9660_LIBS) $(LIBUDF_LIBS) $(LIBCDIO_DEPS)
> +extract_LDADD         = $(LIBISO9660_LIBS) $(LIBUDF_LIBS) $(LIBCDIO_LIBS)
> $(LTLIBICONV)
> +
>  cdio_eject_DEPENDENCIES = $(LIBCDIO_DEPS)
>  cdio_eject_LDADD        = $(LIBCDIO_LIBS) $(LTLIBICONV)
>
> diff --git a/example/extract.c b/example/extract.c
> new file mode 100644
> index 0000000..30644db
> --- /dev/null
> +++ b/example/extract.c
> @@ -0,0 +1,270 @@
> +/*
> +  Copyright (C) 2012 Pete Batard <pete@...>
> +  Based on samples copyright (c) 2003-2011 Rocky Bernstein <rocky@...
> >
> +
> +  This program is free software: you can redistribute it and/or modify
> +  it under the terms of the GNU General Public License as published by
> +  the Free Software Foundation, either version 3 of the License, or
> +  (at your option) any later version.
> +
> +  This program is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +  GNU General Public License for more details.
> +
> +  You should have received a copy of the GNU General Public License
> +  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +/* Extract the full content of either an UDF or ISO9660
> +   TODO: timestamp preservation, file permissions, Unicode
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <malloc.h>
> +#include <errno.h>
> +
> +#include <cdio/cdio.h>
> +#include <cdio/logging.h>
> +#include <cdio/iso9660.h>
> +#include <cdio/udf.h>
> +
> +#if defined(_WIN32)
> +#include <direct.h>
> +#else
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#define _mkdir(a) mkdir(a, S_IRWXU)
> +#endif
> +
> +#ifndef MIN
> +#define MIN(a,b) (((a) < (b)) ? (a) : (b))
> +#endif
> +
> +#define print_vd_info(title, fn)     \
> +  if (fn(p_iso, &psz_str)) {         \
> +    printf(title ": %s\n", psz_str); \
> +  }                                  \
> +  free(psz_str);                     \
> +  psz_str = NULL;
> +
> +const char *psz_extract_dir;
> +
> +static int udf_extract_files(udf_t *p_udf, udf_dirent_t *p_udf_dirent,
> const char *psz_path)
> +{
> +  FILE *fd = NULL;
> +  int i_length;
> +  char* psz_fullpath;
> +  const char* psz_basename;
> +  udf_dirent_t *p_udf_dirent2;
> +  uint8_t buf[UDF_BLOCKSIZE];
> +  int64_t i_read, i_file_length;
> +
> +  if ((p_udf_dirent == NULL) || (psz_path == NULL))
> +    return 1;
> +
> +  while (udf_readdir(p_udf_dirent)) {
> +    psz_basename = udf_get_filename(p_udf_dirent);
> +    i_length = 3 + strlen(psz_path) + strlen(psz_basename) +
> strlen(psz_extract_dir);
> +    psz_fullpath = (char*)calloc(sizeof(char), i_length);
> +    if (psz_fullpath == NULL) {
> +      fprintf(stderr, "Error allocating file name\n");
> +      goto out;
> +    }
> +    i_length = snprintf(psz_fullpath, i_length, "%s%s/%s",
> psz_extract_dir, psz_path, psz_basename);
> +    if (i_length < 0) {
> +      goto out;
> +    }
> +    printf("Extracting: %s\n", psz_fullpath);
> +    if (udf_is_dir(p_udf_dirent)) {
> +      _mkdir(psz_fullpath);
> +      p_udf_dirent2 = udf_opendir(p_udf_dirent);
> +      if (p_udf_dirent2 != NULL) {
> +        if (udf_extract_files(p_udf, p_udf_dirent2,
> &psz_fullpath[strlen(psz_extract_dir)]))
> +          goto out;
> +      }
> +    } else {
> +      fd = fopen(psz_fullpath, "wb");
> +      if (fd == NULL) {
> +        fprintf(stderr, "  Unable to create file\n");
> +        goto out;
> +      }
> +      i_file_length = udf_get_file_length(p_udf_dirent);
> +      while (i_file_length > 0) {
> +        memset(buf, 0, UDF_BLOCKSIZE);
> +        i_read = udf_read_block(p_udf_dirent, buf, 1);
> +        if (i_read < 0) {
> +          fprintf(stderr, "  Error reading UDF file %s\n",
> &psz_fullpath[strlen(psz_extract_dir)]);
> +          goto out;
> +        }
> +        fwrite(buf, (size_t)MIN(i_file_length, i_read), 1, fd);
> +        if (ferror(fd)) {
> +          fprintf(stderr, "  Error writing file\n");
> +          goto out;
> +        }
> +        i_file_length -= i_read;
> +      }
> +      fclose(fd);
> +      fd = NULL;
> +    }
> +    free(psz_fullpath);
> +  }
> +  return 0;
> +
> +out:
> +  if (fd != NULL)
> +    fclose(fd);
> +  free(psz_fullpath);
> +  return 1;
> +}
> +
> +static int iso_extract_files(iso9660_t* p_iso, const char *psz_path)
> +{
> +  FILE *fd = NULL;
> +  int i_length, r = 1;
> +  char psz_fullpath[4096], *psz_basename;
> +  const char *psz_iso_name = &psz_fullpath[strlen(psz_extract_dir)];
> +  unsigned char buf[ISO_BLOCKSIZE];
> +  CdioListNode_t* p_entnode;
> +  iso9660_stat_t *p_statbuf;
> +  CdioList_t* p_entlist;
> +  size_t i;
> +  lsn_t lsn;
> +  int64_t i_file_length;
> +
> +  if ((p_iso == NULL) || (psz_path == NULL))
> +    return 1;
> +
> +  i_length = snprintf(psz_fullpath, sizeof(psz_fullpath), "%s%s/",
> psz_extract_dir, psz_path);
> +  if (i_length < 0)
> +    return 1;
> +  psz_basename = &psz_fullpath[i_length];
> +
> +  p_entlist = iso9660_ifs_readdir(p_iso, psz_path);
> +  if (!p_entlist)
> +    return 1;
> +
> +  _CDIO_LIST_FOREACH (p_entnode, p_entlist) {
> +    p_statbuf = (iso9660_stat_t*) _cdio_list_node_data(p_entnode);
> +    /* Eliminate . and .. entries */
> +    if ( (strcmp(p_statbuf->filename, ".") == 0)
> +      || (strcmp(p_statbuf->filename, "..") == 0) )
> +      continue;
> +    iso9660_name_translate(p_statbuf->filename, psz_basename);
> +    if (p_statbuf->type == _STAT_DIR) {
> +      _mkdir(psz_fullpath);
> +      if (iso_extract_files(p_iso, psz_iso_name))
> +        goto out;
> +    } else {
> +      printf("Extracting: %s\n", psz_fullpath);
> +      fd = fopen(psz_fullpath, "wb");
> +      if (fd == NULL) {
> +        fprintf(stderr, "  Unable to create file\n");
> +        goto out;
> +      }
> +      i_file_length = p_statbuf->size;
> +      for (i = 0; i_file_length > 0; i++) {
> +        memset(buf, 0, ISO_BLOCKSIZE);
> +        lsn = p_statbuf->lsn + i;
> +        if (iso9660_iso_seek_read(p_iso, buf, lsn, 1) != ISO_BLOCKSIZE) {
> +          fprintf(stderr, "  Error reading ISO9660 file %s at LSN %lu\n",
> +            psz_iso_name, (long unsigned int)lsn);
> +          goto out;
> +        }
> +        fwrite(buf, (size_t)MIN(i_file_length, ISO_BLOCKSIZE), 1, fd);
> +        if (ferror(fd)) {
> +          fprintf(stderr, "  Error writing file\n");
> +          goto out;
> +        }
> +        i_file_length -= ISO_BLOCKSIZE;
> +      }
> +      fclose(fd);
> +      fd = NULL;
> +    }
> +  }
> +  r = 0;
> +
> +out:
> +  if (fd != NULL)
> +    fclose(fd);
> +  _cdio_list_free(p_entlist, true);
> +  return r;
> +}
> +
> +int main(int argc, char** argv)
> +{
> +  iso9660_t* p_iso = NULL;
> +  udf_t* p_udf = NULL;
> +  udf_dirent_t* p_udf_root;
> +  char *psz_str = NULL;
> +  char vol_id[UDF_VOLID_SIZE] = "";
> +  char volset_id[UDF_VOLSET_ID_SIZE+1] = "";
> +  int r = 0;
> +
> +  cdio_loglevel_default = CDIO_LOG_DEBUG;
> +
> +  if (argc < 3) {
> +    fprintf(stderr, "Usage: extract <iso_image> <extraction_dir>\n");
> +    return 1;
> +  }
> +
> +  psz_extract_dir = argv[2];
> +  if (_mkdir(psz_extract_dir) == 0) {
> +    printf("Creating directory: %s\n", psz_extract_dir);
> +  } else if (errno != EEXIST) {
> +    fprintf(stderr, "Unable to create extraction directory %s\n",
> psz_extract_dir);
> +    return 1;
> +  }
> +
> +  /* First try to open as UDF - fallback to ISO if it failed */
> +  p_udf = udf_open(argv[1]);
> +  if (p_udf == NULL)
> +    goto try_iso;
> +
> +  p_udf_root = udf_get_root(p_udf, true, 0);
> +  if (p_udf_root == NULL) {
> +    fprintf(stderr, "Couldn't locate UDF root directory\n");
> +    goto out;
> +  }
> +  vol_id[0] = 0; volset_id[0] = 0;
> +
> +  /* Show basic UDF Volume info */
> +  if (udf_get_volume_id(p_udf, vol_id, sizeof(vol_id)) > 0)
> +    fprintf(stderr, "Volume id: %s\n", vol_id);
> +  if (udf_get_volume_id(p_udf, volset_id, sizeof(volset_id)) >0 ) {
> +    volset_id[UDF_VOLSET_ID_SIZE]='\0';
> +    fprintf(stderr, "Volume set id: %s\n", volset_id);
> +  }
> +  fprintf(stderr, "Partition number: %d\n", udf_get_part_number(p_udf));
> +
> +  /* Recursively extract files */
> +  r = udf_extract_files(p_udf, p_udf_root, "");
> +
> +  goto out;
> +
> +try_iso:
> +  p_iso = iso9660_open(argv[1]);
> +  if (p_iso == NULL) {
> +    fprintf(stderr, "Unable to open image '%s'.\n", argv[1]);
> +    goto out;
> +  }
> +
> +  /* Show basic ISO9660 info from the Primary Volume Descriptor. */
> +  print_vd_info("Application", iso9660_ifs_get_application_id);
> +  print_vd_info("Preparer   ", iso9660_ifs_get_preparer_id);
> +  print_vd_info("Publisher  ", iso9660_ifs_get_publisher_id);
> +  print_vd_info("System     ", iso9660_ifs_get_system_id);
> +  print_vd_info("Volume     ", iso9660_ifs_get_volume_id);
> +  print_vd_info("Volume Set ", iso9660_ifs_get_volumeset_id);
> +
> +  r = iso_extract_files(p_iso, "");
> +
> +out:
> +  if (p_iso != NULL)
> +    iso9660_close(p_iso);
> +  if (p_udf != NULL)
> +    udf_close(p_udf);
> +
> +  return r;
> +}
> --
> 1.7.8.msysgit.0
>
>
>

[r-]

On Sun, Jan 22, 2012 at 7:01 PM, Pete Batard <pbatard@...> wrote:

> Alright, now that I have straightened out a working UDF+ISO extraction in
> my application (or at least what looks like one so far), I am in a position
> to try to feed back a few fixes and improvements to libcdio.
>

Great!


>
> Naturally, I don't expect all the modifications I have currently applied
> to the source to work in my app, to be transposable as is, especially as I
> took some liberties here and there. But at the very leats, I should be able
> to provide details on how I addressed the specific issues that I had, and
> we can try to take it from there.
>

If it improves your usage it will probably improve other's usage. From what
I've seen so far from the changes I don't see way things are going to get
worse for people.


>
> Overall, even though I almost gave up on libcdio in despair a few days
> ago, not much seems to be needed to get UDF support going.
>

Good to hear.

>
> For people interested in testing the upcoming changes, the first thing we
> are going to need is a new sample that does image extraction, which is
> provided in the attached patch. It's heavily based on what the existing iso
> and udf samples already do for directory listing and file extraction, the
> main different being that it avoid using ftruncate because ultimately we
> would need an ftruncate64 (we'll be handling some files that are > 4GB in
> size) and this becomes a cross-platform headache, especially if one plans
> to eventually support MSVC compilers.
> The current sample does not do timestamp/premissions preservation and does
> not deal with Unicode (unless the underlying calls support UTF-8, which
> wouldn't be the case on Windows), as this isn't really something we should
> have issues with. One I have sorted these in my app, I'll probably update
> the sample though.
>

Sample programs don't have to be perfect and handle all cases. On the other
hand, if there is something that is going to have to be coded again and
again, then perhaps the code should be put in the library.



>
> Before I go ahead an apply it to mainline, I'd appreciate if you could
> review the sample to confirm that it looks OK. Unless I get a green light,
> I'm not planning to push anything to mainline for another day or two.
>

To make it easier for others to test things out, I've created a pbatard
branch and committed the patch here and the next one to reset
_udf->i_position.

You can commit future changes to the pbatard branch, and then when folks
feel the time is read this can be merged in the master branch.

>
> The second item we will need besides that sample, is an UDF image that can
> exhibit issues that need to be addressed. The one I would encourage
> everyone to use would be the "Windows 8 Developer Preview with developer
> tools English" since:
> 1. It is freely available from Microsoft
> 2. It is more than 4 GB in size (=> will test 32 bit limitations)
> 3. It also contains a file that is more than 4 GB in size (=> more 32 bit
> test, as well as extended attributes use)
> This image can be downloaded at: [1] (direct link [2]).
>

We can't include this in the distribution. In fact anything that is 4GB in
size can't be included in the distribution.

Conceivably one could add some sort of Makefile target to issue a
retrieval. Then there is the thing that is unsettling to me that this link
could be broken at any time.

Independent of that one wants the ability to test code. I don't think it
reasonable to ask people to download 4GB just to test UDF handling,
although it may be fine to suggest that for more complete or realistic
tests.

Specifically, it is not clear that one couldn't come up with a very small
UDF image that exhibits the _udf->i_position problem of your next patch.

> With the sample and the image file above, we should then be able to:
>
> 1. Make UDF extraction work on Linux 32 bit (note that 64 bit Linux may
> not exhibit the 32 bit problems I've seen, so I'd strongly encourage to use
> 32 bit Linux for tests)
>
> 2. Make UDF extraction work on MinGW32 as well as other platforms that
> don't have _FILE_OFFSET_BITS 64 or transparent large file support
>
> 3. Make UDF and ISO9660 image extraction work with MSVC on Windows
>
>
> As you may guess, my objective is to bridge as much as the gap as possible
> between the libcdio source I use in my app (which is compiled with both
> MinGW and MSVC), and official. Eventually, I'd like to be able to use
> mainline files as is, to easily benefit from future libcdio fixes/updates.
>
> Finally, because I am only interested in image extraction at the moment,
> and it should be enough to keep us busy for some time (especially if we add
> MSVC support), I'm going to be very restrictive in the scope of libcdio for
> this whole exercise.

Perfectly reasonable. I and perhaps others will test with the other options.


> In effect, the following options are what I'll use on all platforms (which
> you may want to add to your autogen.sh for testing)
>
> ./configure --enable-maintainer-mode --disable-cddb \
>  --disable-vcd-info --disable-cxx --disable-cpp-progs \
>  --without-cd-drive --without-cd-info --without-cd-paranoia \
>  --without-cdda-player --without-cd-read

> With this sorted out, we are ready to start patching libcdio...
>
> Regards,
>
> /Pete
>
> [1] http://msdn.microsoft.com/en-**us/windows/apps/br229516<http://msdn.microsoft.com/en-us/windows/apps/br229516>
> [2] http://wdp.dlws.microsoft.com/**WDPDL/**9B8DFDFF736C5B1DBF956B89D8A9D4
> **FD925DACD2/**WindowsDeveloperPreview-64bit-**English-Developer.iso<http://wdp.dlws.microsoft.com/WDPDL/9B8DFDFF736C5B1DBF956B89D8A9D4FD925DACD2/WindowsDeveloperPreview-64bit-English-Developer.iso>
>
> From 4616028005dfedc6580baa75d5390a9696921ffc Mon Sep 17 00:00:00 2001
> From: Pete Batard <pete@...>
> Date: Sun, 22 Jan 2012 02:00:37 +0000
> Subject: [PATCH] Add ISO9660+UDF image extraction example
>
> ---
>  example/Makefile.am |    5 +-
>  example/extract.c   |  270
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 274 insertions(+), 1 deletions(-)
>  create mode 100644 example/extract.c
>
> diff --git a/example/Makefile.am b/example/Makefile.am
> index de65b5b..995f619 100644
> --- a/example/Makefile.am
> +++ b/example/Makefile.am
> @@ -26,7 +26,7 @@ paranoia_progs = paranoia paranoia2
>  endif
>  if BUILD_EXAMPLES
>  noinst_PROGRAMS = audio cdchange cdtext device discid drives eject \
> -                 isofile isofile2 isofuzzy isolist isolsn \
> +                 extract isofile isofile2 isofuzzy isolist isolsn \
>                  mmc1 mmc2 mmc2a mmc3 $(paranoia_progs) tracks \
>                  sample3 sample4 udf1 udffile cdio-eject
>  endif
> @@ -54,6 +54,9 @@ drives_LDADD          = $(LIBCDIO_LIBS) $(LTLIBICONV)
>  eject_DEPENDENCIES    = $(LIBCDIO_DEPS)
>  eject_LDADD           = $(LIBCDIO_LIBS) $(LTLIBICONV)
>
> +extract_DEPENDENCIES  = $(LIBISO9660_LIBS) $(LIBUDF_LIBS) $(LIBCDIO_DEPS)
> +extract_LDADD         = $(LIBISO9660_LIBS) $(LIBUDF_LIBS) $(LIBCDIO_LIBS)
> $(LTLIBICONV)
> +
>  cdio_eject_DEPENDENCIES = $(LIBCDIO_DEPS)
>  cdio_eject_LDADD        = $(LIBCDIO_LIBS) $(LTLIBICONV)
>
> diff --git a/example/extract.c b/example/extract.c
> new file mode 100644
> index 0000000..30644db
> --- /dev/null
> +++ b/example/extract.c
> @@ -0,0 +1,270 @@
> +/*
> +  Copyright (C) 2012 Pete Batard <pete@...>
> +  Based on samples copyright (c) 2003-2011 Rocky Bernstein <rocky@...
> >
> +
> +  This program is free software: you can redistribute it and/or modify
> +  it under the terms of the GNU General Public License as published by
> +  the Free Software Foundation, either version 3 of the License, or
> +  (at your option) any later version.
> +
> +  This program is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +  GNU General Public License for more details.
> +
> +  You should have received a copy of the GNU General Public License
> +  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +/* Extract the full content of either an UDF or ISO9660
> +   TODO: timestamp preservation, file permissions, Unicode
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <malloc.h>
> +#include <errno.h>
> +
> +#include <cdio/cdio.h>
> +#include <cdio/logging.h>
> +#include <cdio/iso9660.h>
> +#include <cdio/udf.h>
> +
> +#if defined(_WIN32)
> +#include <direct.h>
> +#else
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#define _mkdir(a) mkdir(a, S_IRWXU)
> +#endif
> +
> +#ifndef MIN
> +#define MIN(a,b) (((a) < (b)) ? (a) : (b))
> +#endif
> +
> +#define print_vd_info(title, fn)     \
> +  if (fn(p_iso, &psz_str)) {         \
> +    printf(title ": %s\n", psz_str); \
> +  }                                  \
> +  free(psz_str);                     \
> +  psz_str = NULL;
> +
> +const char *psz_extract_dir;
> +
> +static int udf_extract_files(udf_t *p_udf, udf_dirent_t *p_udf_dirent,
> const char *psz_path)
> +{
> +  FILE *fd = NULL;
> +  int i_length;
> +  char* psz_fullpath;
> +  const char* psz_basename;
> +  udf_dirent_t *p_udf_dirent2;
> +  uint8_t buf[UDF_BLOCKSIZE];
> +  int64_t i_read, i_file_length;
> +
> +  if ((p_udf_dirent == NULL) || (psz_path == NULL))
> +    return 1;
> +
> +  while (udf_readdir(p_udf_dirent)) {
> +    psz_basename = udf_get_filename(p_udf_dirent);
> +    i_length = 3 + strlen(psz_path) + strlen(psz_basename) +
> strlen(psz_extract_dir);
> +    psz_fullpath = (char*)calloc(sizeof(char), i_length);
> +    if (psz_fullpath == NULL) {
> +      fprintf(stderr, "Error allocating file name\n");
> +      goto out;
> +    }
> +    i_length = snprintf(psz_fullpath, i_length, "%s%s/%s",
> psz_extract_dir, psz_path, psz_basename);
> +    if (i_length < 0) {
> +      goto out;
> +    }
> +    printf("Extracting: %s\n", psz_fullpath);
> +    if (udf_is_dir(p_udf_dirent)) {
> +      _mkdir(psz_fullpath);
> +      p_udf_dirent2 = udf_opendir(p_udf_dirent);
> +      if (p_udf_dirent2 != NULL) {
> +        if (udf_extract_files(p_udf, p_udf_dirent2,
> &psz_fullpath[strlen(psz_extract_dir)]))
> +          goto out;
> +      }
> +    } else {
> +      fd = fopen(psz_fullpath, "wb");
> +      if (fd == NULL) {
> +        fprintf(stderr, "  Unable to create file\n");
> +        goto out;
> +      }
> +      i_file_length = udf_get_file_length(p_udf_dirent);
> +      while (i_file_length > 0) {
> +        memset(buf, 0, UDF_BLOCKSIZE);
> +        i_read = udf_read_block(p_udf_dirent, buf, 1);
> +        if (i_read < 0) {
> +          fprintf(stderr, "  Error reading UDF file %s\n",
> &psz_fullpath[strlen(psz_extract_dir)]);
> +          goto out;
> +        }
> +        fwrite(buf, (size_t)MIN(i_file_length, i_read), 1, fd);
> +        if (ferror(fd)) {
> +          fprintf(stderr, "  Error writing file\n");
> +          goto out;
> +        }
> +        i_file_length -= i_read;
> +      }
> +      fclose(fd);
> +      fd = NULL;
> +    }
> +    free(psz_fullpath);
> +  }
> +  return 0;
> +
> +out:
> +  if (fd != NULL)
> +    fclose(fd);
> +  free(psz_fullpath);
> +  return 1;
> +}
> +
> +static int iso_extract_files(iso9660_t* p_iso, const char *psz_path)
> +{
> +  FILE *fd = NULL;
> +  int i_length, r = 1;
> +  char psz_fullpath[4096], *psz_basename;
> +  const char *psz_iso_name = &psz_fullpath[strlen(psz_extract_dir)];
> +  unsigned char buf[ISO_BLOCKSIZE];
> +  CdioListNode_t* p_entnode;
> +  iso9660_stat_t *p_statbuf;
> +  CdioList_t* p_entlist;
> +  size_t i;
> +  lsn_t lsn;
> +  int64_t i_file_length;
> +
> +  if ((p_iso == NULL) || (psz_path == NULL))
> +    return 1;
> +
> +  i_length = snprintf(psz_fullpath, sizeof(psz_fullpath), "%s%s/",
> psz_extract_dir, psz_path);
> +  if (i_length < 0)
> +    return 1;
> +  psz_basename = &psz_fullpath[i_length];
> +
> +  p_entlist = iso9660_ifs_readdir(p_iso, psz_path);
> +  if (!p_entlist)
> +    return 1;
> +
> +  _CDIO_LIST_FOREACH (p_entnode, p_entlist) {
> +    p_statbuf = (iso9660_stat_t*) _cdio_list_node_data(p_entnode);
> +    /* Eliminate . and .. entries */
> +    if ( (strcmp(p_statbuf->filename, ".") == 0)
> +      || (strcmp(p_statbuf->filename, "..") == 0) )
> +      continue;
> +    iso9660_name_translate(p_statbuf->filename, psz_basename);
> +    if (p_statbuf->type == _STAT_DIR) {
> +      _mkdir(psz_fullpath);
> +      if (iso_extract_files(p_iso, psz_iso_name))
> +        goto out;
> +    } else {
> +      printf("Extracting: %s\n", psz_fullpath);
> +      fd = fopen(psz_fullpath, "wb");
> +      if (fd == NULL) {
> +        fprintf(stderr, "  Unable to create file\n");
> +        goto out;
> +      }
> +      i_file_length = p_statbuf->size;
> +      for (i = 0; i_file_length > 0; i++) {
> +        memset(buf, 0, ISO_BLOCKSIZE);
> +        lsn = p_statbuf->lsn + i;
> +        if (iso9660_iso_seek_read(p_iso, buf, lsn, 1) != ISO_BLOCKSIZE) {
> +          fprintf(stderr, "  Error reading ISO9660 file %s at LSN %lu\n",
> +            psz_iso_name, (long unsigned int)lsn);
> +          goto out;
> +        }
> +        fwrite(buf, (size_t)MIN(i_file_length, ISO_BLOCKSIZE), 1, fd);
> +        if (ferror(fd)) {
> +          fprintf(stderr, "  Error writing file\n");
> +          goto out;
> +        }
> +        i_file_length -= ISO_BLOCKSIZE;
> +      }
> +      fclose(fd);
> +      fd = NULL;
> +    }
> +  }
> +  r = 0;
> +
> +out:
> +  if (fd != NULL)
> +    fclose(fd);
> +  _cdio_list_free(p_entlist, true);
> +  return r;
> +}
> +
> +int main(int argc, char** argv)
> +{
> +  iso9660_t* p_iso = NULL;
> +  udf_t* p_udf = NULL;
> +  udf_dirent_t* p_udf_root;
> +  char *psz_str = NULL;
> +  char vol_id[UDF_VOLID_SIZE] = "";
> +  char volset_id[UDF_VOLSET_ID_SIZE+1] = "";
> +  int r = 0;
> +
> +  cdio_loglevel_default = CDIO_LOG_DEBUG;
> +
> +  if (argc < 3) {
> +    fprintf(stderr, "Usage: extract <iso_image> <extraction_dir>\n");
> +    return 1;
> +  }
> +
> +  psz_extract_dir = argv[2];
> +  if (_mkdir(psz_extract_dir) == 0) {
> +    printf("Creating directory: %s\n", psz_extract_dir);
> +  } else if (errno != EEXIST) {
> +    fprintf(stderr, "Unable to create extraction directory %s\n",
> psz_extract_dir);
> +    return 1;
> +  }
> +
> +  /* First try to open as UDF - fallback to ISO if it failed */
> +  p_udf = udf_open(argv[1]);
> +  if (p_udf == NULL)
> +    goto try_iso;
> +
> +  p_udf_root = udf_get_root(p_udf, true, 0);
> +  if (p_udf_root == NULL) {
> +    fprintf(stderr, "Couldn't locate UDF root directory\n");
> +    goto out;
> +  }
> +  vol_id[0] = 0; volset_id[0] = 0;
> +
> +  /* Show basic UDF Volume info */
> +  if (udf_get_volume_id(p_udf, vol_id, sizeof(vol_id)) > 0)
> +    fprintf(stderr, "Volume id: %s\n", vol_id);
> +  if (udf_get_volume_id(p_udf, volset_id, sizeof(volset_id)) >0 ) {
> +    volset_id[UDF_VOLSET_ID_SIZE]='\0';
> +    fprintf(stderr, "Volume set id: %s\n", volset_id);
> +  }
> +  fprintf(stderr, "Partition number: %d\n", udf_get_part_number(p_udf));
> +
> +  /* Recursively extract files */
> +  r = udf_extract_files(p_udf, p_udf_root, "");
> +
> +  goto out;
> +
> +try_iso:
> +  p_iso = iso9660_open(argv[1]);
> +  if (p_iso == NULL) {
> +    fprintf(stderr, "Unable to open image '%s'.\n", argv[1]);
> +    goto out;
> +  }
> +
> +  /* Show basic ISO9660 info from the Primary Volume Descriptor. */
> +  print_vd_info("Application", iso9660_ifs_get_application_id);
> +  print_vd_info("Preparer   ", iso9660_ifs_get_preparer_id);
> +  print_vd_info("Publisher  ", iso9660_ifs_get_publisher_id);
> +  print_vd_info("System     ", iso9660_ifs_get_system_id);
> +  print_vd_info("Volume     ", iso9660_ifs_get_volume_id);
> +  print_vd_info("Volume Set ", iso9660_ifs_get_volumeset_id);
> +
> +  r = iso_extract_files(p_iso, "");
> +
> +out:
> +  if (p_iso != NULL)
> +    iso9660_close(p_iso);
> +  if (p_udf != NULL)
> +    udf_close(p_udf);
> +
> +  return r;
> +}
> --
> 1.7.8.msysgit.0
>
>
>

[Pete Batard]

On 2012.01.23 01:47, Rocky Bernstein wrote:
> To make it easier for others to test things out, I've created a pbatard
> branch and committed the patch here and the next one to reset
> _udf->i_position.
>
> You can commit future changes to the pbatard branch, and then when folks
> feel the time is read this can be merged in the master branch.

Sounds good to me. Thanks!

> We can't include this in the distribution. In fact anything that is 4GB in
> size can't be included in the distribution.

Sorry if I wasn't clear here - I'm only talking about people wanting to
follow on the upcoming UDF fixes using this image *temporarily*, as a
convenience data file. It was not my intention to imply that it should
be provided with the libcdio distro, because of course that makes no sense!

It's just that this specific image displays issues that can help us fix
things here and there, and avoid having to create our own test image.
Once these fixes are in, there's no need for it really.

Regards,

/Pete

[Pete Batard]

Oh, and as I explained, unless we have a >4GB (or >2 GB if all indexes
are signed) image with a >4GB (2GB - signed) file in it, tests for large
files on 32 bit systems will of course not be representative.

I'm afraid figuring out how to create such an image from scratch, when I
know of one freely available, is not part of my current plan. I feel
there is just too much that will keep us busy already, without having to
worry about adding an extra step. I hope that's OK.

Regards,

/Pete

[Pete Batard]

> On Sun, Jan 22, 2012 at 7:01 PM, Pete Batard<pbatard@...>  wrote:
>> In effect, the following options are what I'll use on all platforms (which
>> you may want to add to your autogen.sh for testing)
>>
>> ./configure --enable-maintainer-mode --disable-cddb \
>>   --disable-vcd-info --disable-cxx --disable-cpp-progs \
>>   --without-cd-drive --without-cd-info --without-cd-paranoia \
>>   --without-cdda-player --without-cd-read

For the record, I have now pushed a commit that adds an
'autogen-pbatard.sh' to my branch, to include the options above.

Regards,

/Pete

[r-]

On Mon, Jan 23, 2012 at 5:24 AM, Pete Batard <pbatard@...> wrote:

> On 2012.01.23 01:47, Rocky Bernstein wrote:
>
>> To make it easier for others to test things out, I've created a pbatard
>> branch and committed the patch here and the next one to reset
>> _udf->i_position.
>>
>> You can commit future changes to the pbatard branch, and then when folks
>> feel the time is read this can be merged in the master branch.
>>
>
> Sounds good to me. Thanks!
>
>
>  We can't include this in the distribution. In fact anything that is 4GB in
>> size can't be included in the distribution.
>>
>
> Sorry if I wasn't clear here - I'm only talking about people wanting to
> follow on the upcoming UDF fixes using this image *temporarily*, as a
> convenience data file. It was not my intention to imply that it should be
> provided with the libcdio distro, because of course that makes no sense!
>
> It's just that this specific image displays issues that can help us fix
> things here and there, and avoid having to create our own test image. Once
> these fixes are in, there's no need for it really.
>

Ok. But also see the next reply.

>
> Regards,
>
> /Pete
>
>

[r-]

On Mon, Jan 23, 2012 at 5:35 AM, Pete Batard <pbatard@...> wrote:

> Oh, and as I explained, unless we have a >4GB (or >2 GB if all indexes are
> signed) image with a >4GB (2GB - signed) file in it, tests for large files
> on 32 bit systems will of course not be representative.
>
> I'm afraid figuring out how to create such an image from scratch, when I
> know of one freely available, is not part of my current plan. I feel there
> is just too much that will keep us busy already, without having to worry
> about adding an extra step. I hope that's OK.
>

There is a style of programming development called "test-driven
development" (TDD) [1] and something similar called "behavior-driven
development" (BDD) [2].  Ruby on Rails programmers use it all the time, and
I have found this style of development helpful in code and projects I write.

In this mode, one writes the tests before the code. Sure, you use this real
image to find bugs because that's what you really want to be able to
handle. However after finding the bug one can isolate the bug down to a
specific small test case that exhibits the bug and *then* fix that after
the test is in place.

Yes, this can slightly slow down coding in the same way say as writing
function comments giving a behavioral specification can slow down coding.
Invariably it is a little extra work because things change as one
understands more so that means updating comments and tests.

However, if you look at this in the big picture as -- one has to do for
long-lived and large projects -- the tests are essential to ensure we don't
regress and to allow people to have some confidence that this code does
what it is supposed to do in their environment.

I am of the frame of mind that I write *some* tests and comments as I go
along rather than do *everything* after the fact. There may be a stage
where I tidy up code, comments and improve tests. And by doing so, I can
ensure no regression of the things I am focused on while I am developing
the code.

Specifically here, you found a problem where it looks like sequential
reading of files is buggy. This thing where a variable should be reset to 0
after use or before the next read. It strikes me that one should be able to
write a small UDF image to exhibit that behavior. It might just be as
simple as UDF with two or three files in it.

[1] http://en.wikipedia.org/wiki/Test-driven_development
[2] http://en.wikipedia.org/wiki/Behavior_Driven_Development


> Regards,
>
> /Pete
>
>

[Pete Batard]

On 2012.01.23 12:35, Rocky Bernstein wrote:
> There is a style of programming development called "test-driven
> development" (TDD) [1] and something similar called "behavior-driven
> development" (BDD) [2].  Ruby on Rails programmers use it all the time, and
> I have found this style of development helpful in code and projects I write.

Yes. I've done ISO 9000 development in my time as well, where unit test
and development go hand in hand, and you're not supposed to code a
feature without first having a functional test for it (or at least the
specs for a functional test, which you then have to implement). I also
saw the time such a process can incur first hand...

> In this mode, one writes the tests before the code.

Well, as you can guess, my problem here is time.

This will just be too time consuming as far as I'm concerned, and I'm
already going beyond my original plan, by delaying further development
of my app as well as the other projects I'm involved with, to help
libcdio. Helping libcdio is not something that was on my radar at all
when I decided to add iso image support to my app (because I thought I'd
just reuse 7-zip, which doesn't exhibit any of these issues). Thus, my
original plan was to spend about one week or so on the whole feature,
before moving onto something else... Famous last words.
Therefore, I'm already way over my allocated time budget here and
effectively trying to reduce this unforeseen cost. But I still very much
want to help libcdio as much as I can!

Now, if I had an employer paying for the FOSS development I do, I'd like
nothing more than help out and spend time writing unit tests for
libcdio, because I agree it'd be for the best. But I don't, so my time
is limited. I already haven't gotten a chance to write unit tests with
the other projects I'm involved with (for instance we could really use a
multithreading sample in libusb), so I don't really see how I can
suddenly justify doing so for libcdio.

Also, if we didn't have anything public to help reproduce the issues, I
would agree that it'd be troublesome. However we do: the Windows image I
pointed out does exhibit all the symptoms we want to fix. Therefore,
creating unit test is mostly duplication of something we already have,
and sounds like a very avoidable constraint.

> Yes, this can slightly slow down coding in the same way say as writing
> function comments giving a behavioral specification can slow down coding.
> Invariably it is a little extra work because things change as one
> understands more so that means updating comments and tests.

I have to disagree that the impact is minimal. I don't know anything
about creating UDF images and barely a thing about the UDF file system.
Doing things properly would require an extra time investment that I
cannot really justify.

> However, if you look at this in the big picture as -- one has to do for
> long-lived and large projects -- the tests are essential to ensure we don't
> regress and to allow people to have some confidence that this code does
> what it is supposed to do in their environment.

My worry is that, if we actually wanted to do things properly for UDF,
we shouldn't limit ourselves to just the Windows image issue
replication. We'd need to add an Unicode test, a 4096(?) strategy test
as well as review the various FIXME's in the code and so on. This is the
only way I see to "allow people to have some confidence".
Ergo, since I do not see myself committing the time required for such
additional tasks, and I don't think you will either, we may have no
choice but take shortcuts here and there.

> Specifically here, you found a problem where it looks like sequential
> reading of files is buggy. This thing where a variable should be reset to 0
> after use or before the next read. It strikes me that one should be able to
> write a small UDF image to exhibit that behavior. It might just be as
> simple as UDF with two or three files in it.

Agreed. One should be able. One who is willing to commit time to write
such a test. I don't see myself being such a person anytime soon, sorry.

I hope this doesn't sound to harsh, but that's the reality of my
commitment to libcdio. Sometimes FOSS developers just have too much on
their hands and need to take shortcuts and prioritize tasks. But because
it's FOSS, I don't see that as an entirely unreasonable thing to do.

So I will finish by saying that, when developing for FOSS, and as
opposed to proprietary, you're not supposed to be in the woods, at
night, all alone with your code. Instead, you're be in broad daylight
(open) and should be able to rely on others ("with enough eyeballs,
etc."). Therefore, the methodologies that are aimed at proprietary
development, even if they are also most certainly helpful when applied
to FOSS as is, should allow some tweaking here and there, such as a bit
of leeway for unit tests, if the end result is that more people will
contribute.

Regards,

/Pete

[r-]

On Mon, Jan 23, 2012 at 9:23 AM, Pete Batard <pbatard@...> wrote:

> On 2012.01.23 12:35, Rocky Bernstein wrote:
>
>> There is a style of programming development called "test-driven
>> development" (TDD) [1] and something similar called "behavior-driven
>> development" (BDD) [2].  Ruby on Rails programmers use it all the time,
>> and
>> I have found this style of development helpful in code and projects I
>> write.
>>
>
> Yes. I've done ISO 9000 development in my time as well, where unit test
> and development go hand in hand, and you're not supposed to code a feature
> without first having a functional test for it (or at least the specs for a
> functional test, which you then have to implement). I also saw the time
> such a process can incur first hand...

Anything can be overdone. Part of the art is the right level of testing or
comments.  It is not black or white as to whether something is tested or
commented. And as you suggest, different for projects and different
considerations the level changes.

>
>
>  In this mode, one writes the tests before the code.
>>
>
> Well, as you can guess, my problem here is time.
>
> This will just be too time consuming as far as I'm concerned, and I'm
> already going beyond my original plan, by delaying further development of
> my app as well as the other projects I'm involved with, to help libcdio.
> Helping libcdio is not something that was on my radar

And UDF support wasn't on mine. But as you write below, famous last words.
Many of the projects I've gotten involved with have gone in lots of
directions I never foresaw. But that's probably a good thing - the road
less travelled.


> at all when I decided to add iso image support to my app (because I
> thought I'd just reuse 7-zip, which doesn't exhibit any of these issues).
> Thus, my original plan was to spend about one week or so on the whole
> feature, before moving onto something else...
>
Famous last words.
> Therefore, I'm already way over my allocated time budget here and
> effectively trying to reduce this unforeseen cost. But I still very much
> want to help libcdio as much as I can!
>
> Now, if I had an employer paying for the FOSS development I do, I'd like
> nothing more than help out and spend time writing unit tests for libcdio,
> because I agree it'd be for the best.


Ok. Since I think it important, unless there are other volunteers I'll look
into adding tests for the very specific bugs that are encountered.

There is nothing in TDD that says one has to support all of UDF. I'm taking
a more pragmatic approach here. The are specific bugs and deficiencies
you've encountered with libcdio are the ones we want to make sure are
addressed forever.

And as I wrote before, having even slightly better UDF support may be just
enough to lure more people to handle all those other more thorny issues.



> But I don't, so my time is limited. I already haven't gotten a chance to
> write unit tests with the other projects I'm involved with (for instance we
> could really use a multithreading sample in libusb), so I don't really see
> how I can suddenly justify doing so for libcdio.
>

I understand where you are coming from, but suggest even in those other
projects it may be a mistake. With testing there is a chicken-and-egg
bootstrap problem. Unless one is working in an environment like
Ruby-on-Rails where there already has been a lot of work done to support
testing (even to the extent of automatically cloning databases), there is
some initial hit one takes in setting up tests. But after doing that, the
testing becomes easier. And the pay-off in my view is worth the work done.

Here is something similar. You just wrote an example program. It was
probably easier to do because there were other example programs and the
build framework to cut and paste from. Experience has shown that the
example programs have been very helpful, although when I wrote the first
ones, it felt more as an afterthought.

So again, perhaps if a framework for testing UDF images is started adding
to that for bugs that you want to fix won't be too much trouble. And there
may be some code in the testing that you can use on the other projects.

>
> Also, if we didn't have anything public to help reproduce the issues, I
> would agree that it'd be troublesome. However we do: the Windows image I
> pointed out does exhibit all the symptoms we want to fix. Therefore,
> creating unit test is mostly duplication of something we already have, and
> sounds like a very avoidable constraint.
>
>
>  Yes, this can slightly slow down coding in the same way say as writing
>> function comments giving a behavioral specification can slow down coding.
>> Invariably it is a little extra work because things change as one
>> understands more so that means updating comments and tests.
>>
>
> I have to disagree that the impact is minimal. I don't know anything about
> creating UDF images and barely a thing about the UDF file system. Doing
> things properly would require an extra time investment that I cannot really
> justify.
>
>
>  However, if you look at this in the big picture as -- one has to do for
>> long-lived and large projects -- the tests are essential to ensure we
>> don't
>> regress and to allow people to have some confidence that this code does
>> what it is supposed to do in their environment.
>>
>
> My worry is that, if we actually wanted to do things properly for UDF, we
> shouldn't limit ourselves to just the Windows image issue replication. We'd
> need to add an Unicode test, a 4096(?) strategy test as well as review the
> various FIXME's in the code and so on. This is the only way I see to "allow
> people to have some confidence".
> Ergo, since I do not see myself committing the time required for such
> additional tasks, and I don't think you will either, we may have no choice
> but take shortcuts here and there.
>
>
>  Specifically here, you found a problem where it looks like sequential
>> reading of files is buggy. This thing where a variable should be reset to
>> 0
>> after use or before the next read. It strikes me that one should be able
>> to
>> write a small UDF image to exhibit that behavior. It might just be as
>> simple as UDF with two or three files in it.
>>
>
> Agreed. One should be able. One who is willing to commit time to write
> such a test. I don't see myself being such a person anytime soon, sorry.
>
> I hope this doesn't sound to harsh, but that's the reality of my
> commitment to libcdio. Sometimes FOSS developers just have too much on
> their hands and need to take shortcuts and prioritize tasks. But because
> it's FOSS, I don't see that as an entirely unreasonable thing to do.
>
> So I will finish by saying that, when developing for FOSS, and as opposed
> to proprietary, you're not supposed to be in the woods, at night, all alone
> with your code. Instead, you're be in broad daylight (open) and should be
> able to rely on others ("with enough eyeballs, etc."). Therefore, the
> methodologies that are aimed at proprietary development, even if they are
> also most certainly helpful when applied to FOSS as is, should allow some
> tweaking here and there, such as a bit of leeway for unit tests, if the end
> result is that more people will contribute.
>
> Regards,
>
> /Pete
>
>

[Pete Batard]

On 2012.01.23 15:03, Rocky Bernstein wrote:
> Ok. Since I think it important, unless there are other volunteers I'll look
> into adding tests for the very specific bugs that are encountered.

I very much appreciate that! And I'll try to help as much as I can.

To give you an idea of what's in store, here are the 4 tests we would
need to have an image for then:
- multiple files that can be extracted sequentially (hopefully easy)
- at least one file with extended attributes and allocated descriptors
that result in a File Entry that is greater in size than the last one
used. I'm not exactly sure how the reuse is supposed to occur, and since
it doesn't matter with my proposed fix, I haven't investigated, but this
may be linked to the next test below, as the problem manifests itself
with files that are larger than 1 GB apparently
- at least one file that uses multiple extents (or whatever the UDF
specs calls it). In the case of the Windows UDF, a new extent is used
for every 1 GB of data. Now, my tests with the Windows UDF show that the
current libcdio handles it properly (good job!), but we'd still probably
want that in an image.
- at least one file that is larger than 4 GB. Most likely 2 GB will do,
as I think all the 32 bit issues I spotted were with signed values, but
we might as well go for 4 to be safe.

Additionally, having some filenames in CJK for instance, to
test/implement Unicode preservation would be welcome (I assume most of
us will be testing with a Western locale).

> And as I wrote before, having even slightly better UDF support may be just
> enough to lure more people to handle all those other more thorny issues.

That's what I'm hoping as well, as I'd rather benefit from other people
from fixing issues that I may end up experiencing otherwise ;)

> I understand where you are coming from, but suggest even in those other
> projects it may be a mistake. With testing there is a chicken-and-egg
> bootstrap problem. Unless one is working in an environment like
> Ruby-on-Rails where there already has been a lot of work done to support
> testing (even to the extent of automatically cloning databases), there is
> some initial hit one takes in setting up tests. But after doing that, the
> testing becomes easier. And the pay-off in my view is worth the work done.

Agreed. That also has a lot to do on whether you are in for the short or
the long term.

> Here is something similar. You just wrote an example program. It was
> probably easier to do because there were other example programs and the
> build framework to cut and paste from.

Absolutely! Most of the extract.c sample is copy/paste of existing
libcdio samples, and I'm very grateful for that. However, the main issue
here is not sample accessibility but accessibility of data to test the
samples against, in order to check for potential problems.

My view, and this is also part of the reason I can't see myself spending
much time crafting an UDF test image, is that even after Microsoft
removes the Windows 8 preview UDF from public availability, an awful lot
of libcdio users should still be able to get their hands on a Windows
installation image to display the same (potential) symptoms. All the
Windows 7 images I tested with do and I fully expect the release Windows
8 media to do as well.

Of course, if no public Windows image is available from Microsoft or
elsewhere, I'm not too happy about the idea of punishing libcdio users
that work with Linux, Solaris, BSD, OSX, etc exclusively. But the sheer
market penetration of Windows means that most of these users should be
able to find someone who can provide one.

> Experience has shown that the
> example programs have been very helpful, although when I wrote the first
> ones, it felt more as an afterthought.

Yeah. The other side of the equation, which I've seen with libusb, can
also occur if, after finding that samples are lacking, you decide to
write a generic one that tests a handful of features, instead of going
with a myriad of samples that perform single but very elementary tests,
and the maintainers of the project decide that having no sample at all
is better than having one that does too much...

As a matter of fact, the extract sample is designed to work for both
ISO9660 and UDF images. While I'm aware that this could be split into an
iso-extract.c and an udf-extract.c, I sure hope you're not going to ask
for that, because (since it isn't that large) I think it'll be more
beneficial for libcdio users this way.

> So again, perhaps if a framework for testing UDF images is started adding
> to that for bugs that you want to fix won't be too much trouble. And there
> may be some code in the testing that you can use on the other projects.

Ack. As I said, my main problem is the time I want to spend. Otherwise,
I'm all for having a reliable testing framework as well.

Regards,

/Pete

[r-]

On Mon, Jan 23, 2012 at 12:04 PM, Pete Batard <pbatard@...> wrote:

> On 2012.01.23 15:03, Rocky Bernstein wrote:
>
>> Ok. Since I think it important, unless there are other volunteers I'll
>> look
>> into adding tests for the very specific bugs that are encountered.
>>
>
> I very much appreciate that! And I'll try to help as much as I can.
>
> To give you an idea of what's in store, here are the 4 tests we would need
> to have an image for then:
> - multiple files that can be extracted sequentially (hopefully easy)
>

This was  what commit 462b417e was about. To get a failing test for this
all I had to do was create a UDF with two files (README and COPYING from
libcdio) using the command:

     mkisofs -R -J -udf -iso-level 3 -o /tmp/test-udf1.iso /tmp/udf-test1

Which adds less than 900k to the untarred distribution.


- at least one file with extended attributes and allocated descriptors that
> result in a File Entry that is greater in size than the last one used. I'm
> not exactly sure how the reuse is supposed to occur, and since it doesn't
> matter with my proposed fix, I haven't investigated, but this may be linked
> to the next test below, as the problem manifests itself with files that are
> larger than 1 GB apparently
>

When there is a specific patch for this, I'll start on a test. Otherwise,
I'd not be sure I would be creating the problem you are seeing.


> - at least one file that uses multiple extents (or whatever the UDF specs
> calls it). In the case of the Windows UDF, a new extent is used for every 1
> GB of data. Now, my tests with the Windows UDF show that the current
> libcdio handles it properly (good job!), but we'd still probably want that
> in an image.
>

I am committing only to testing bugs encountered that are you are fixing,
not bugs one can imagine encountering but haven't encountered.  Others who
are more interested in UDF support are welcome to add suggest other tests.
More below...



> - at least one file that is larger than 4 GB. Most likely 2 GB will do, as
> I think all the 32 bit issues I spotted were with signed values, but we
> might as well go for 4 to be safe.
>

Adding 2GB to the distribution for testing everyone gets is not acceptable.
Thus either the images need to be stored someplace where folks can download
them. Or have a way to create suitable images on the fly.  Either way,
probably something for a little later.


>
> Additionally, having some filenames in CJK for instance, to test/implement
> Unicode preservation would be welcome (I assume most of us will be testing
> with a Western locale).


Is this something that has been observed as being a bug?

I didn't say this as part of the TDD/BDD thing, but part of the "Agile
programming" philosophy is starting out with what you need right now, not
what you imagine you might need. Of course, it is also useful to try not to
code oneself in a corner.

>
>
>  And as I wrote before, having even slightly better UDF support may be just
>> enough to lure more people to handle all those other more thorny issues.
>>
>
> That's what I'm hoping as well, as I'd rather benefit from other people
> from fixing issues that I may end up experiencing otherwise ;)
>
>
>  I understand where you are coming from, but suggest even in those other
>> projects it may be a mistake. With testing there is a chicken-and-egg
>> bootstrap problem. Unless one is working in an environment like
>> Ruby-on-Rails where there already has been a lot of work done to support
>> testing (even to the extent of automatically cloning databases), there is
>> some initial hit one takes in setting up tests. But after doing that, the
>> testing becomes easier. And the pay-off in my view is worth the work done.
>>
>
> Agreed. That also has a lot to do on whether you are in for the short or
> the long term.
>
>
>  Here is something similar. You just wrote an example program. It was
>> probably easier to do because there were other example programs and the
>> build framework to cut and paste from.
>>
>
> Absolutely! Most of the extract.c sample is copy/paste of existing libcdio
> samples, and I'm very grateful for that. However, the main issue here is
> not sample accessibility but accessibility of data to test the samples
> against, in order to check for potential problems.
>
> My view, and this is also part of the reason I can't see myself spending
> much time crafting an UDF test image, is that even after Microsoft removes
> the Windows 8 preview UDF from public availability, an awful lot of libcdio
> users should still be able to get their hands on a Windows installation
> image to display the same (potential) symptoms. All the Windows 7 images I
> tested with do and I fully expect the release Windows 8 media to do as well.
>
> Of course, if no public Windows image is available from Microsoft or
> elsewhere, I'm not too happy about the idea of punishing libcdio users that
> work with Linux, Solaris, BSD, OSX, etc exclusively. But the sheer market
> penetration of Windows means that most of these users should be able to
> find someone who can provide one.
>
>
>  Experience has shown that the
>> example programs have been very helpful, although when I wrote the first
>> ones, it felt more as an afterthought.
>>
>
> Yeah. The other side of the equation, which I've seen with libusb, can
> also occur if, after finding that samples are lacking, you decide to write
> a generic one that tests a handful of features, instead of going with a
> myriad of samples that perform single but very elementary tests, and the
> maintainers of the project decide that having no sample at all is better
> than having one that does too much...
>
> As a matter of fact, the extract sample is designed to work for both
> ISO9660 and UDF images. While I'm aware that this could be split into an
> iso-extract.c and an udf-extract.c, I sure hope you're not going to ask for
> that, because (since it isn't that large) I think it'll be more beneficial
> for libcdio users this way.

extract.c  (even with my one line error-reporting fix) is still very short.
So it doesn't matter if it handles ISO9660 and UDF.

>
>  So again, perhaps if a framework for testing UDF images is started adding
>> to that for bugs that you want to fix won't be too much trouble. And there
>> may be some code in the testing that you can use on the other projects.
>>
>
> Ack. As I said, my main problem is the time I want to spend. Otherwise,
> I'm all for having a reliable testing framework as well.
>
> Regards,
>
> /Pete
>
>

[Pete Batard]

On 2012.01.24 03:50, Rocky Bernstein wrote:
> This was  what commit 462b417e was about. To get a failing test for this
> all I had to do was create a UDF with two files (README and COPYING from
> libcdio) using the command:
>
>       mkisofs -R -J -udf -iso-level 3 -o /tmp/test-udf1.iso /tmp/udf-test1
>
> Which adds less than 900k to the untarred distribution.

Sounds good to me. Thanks.

> - at least one file with extended attributes and allocated descriptors that
>> result in a File Entry that is greater in size than the last one used. I'm
>> not exactly sure how the reuse is supposed to occur, and since it doesn't
>> matter with my proposed fix, I haven't investigated, but this may be linked
>> to the next test below, as the problem manifests itself with files that are
>> larger than 1 GB apparently
>
> When there is a specific patch for this, I'll start on a test. Otherwise,
> I'd not be sure I would be creating the problem you are seeing.

OK. I'll start pushing proposed patches directly in -pbatard then.

> I am committing only to testing bugs encountered that are you are fixing,
> not bugs one can imagine encountering but haven't encountered.  Others who
> are more interested in UDF support are welcome to add suggest other tests.
> More below...

Fair enough.

> Adding 2GB to the distribution for testing everyone gets is not acceptable.
> Thus either the images need to be stored someplace where folks can download
> them. Or have a way to create suitable images on the fly.  Either way,
> probably something for a little later.

OK.

>> Additionally, having some filenames in CJK for instance, to test/implement
>> Unicode preservation would be welcome (I assume most of us will be testing
>> with a Western locale).
>
> Is this something that has been observed as being a bug?

No. I expect it to work, but I haven't had a chance to test it yet. I
was just wondering how much you wanted the test image(s) to cover, and
this seemed simple enough to add.

> I didn't say this as part of the TDD/BDD thing, but part of the "Agile
> programming" philosophy is starting out with what you need right now, not
> what you imagine you might need. Of course, it is also useful to try not to
> code oneself in a corner.

Understood. Thanks for the c1ea555d commit.

Regards,

/Pete