[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: libpst: FTBFS on hurd-i386 (for review)



On Sun, 2012-08-26 at 14:28 +0200, Samuel Thibault wrote:
> Svante Signell, le Sat 25 Aug 2012 19:19:02 +0200, a écrit :
> >      if (png_open) {
> >          png_open = false;
> > -        char fn[PATH_MAX];
> > -        snprintf(fn, sizeof(fn), "page%d.png", ++page_sequence);
> > +        int len = 4 + 11 + 4;
> > +        char *fn = (char*)pst_malloc(len + 1);
> > +        snprintf(fn, len, "page%d.png", ++page_sequence);
> 
> len+1 for snprintf too, otherwise the last allocated character will not
> be used, see man snprintf:
> 
> The  functions  snprintf() and vsnprintf() write at most size bytes
> (including the terminating null byte ('\0')) to str.
> 
> Or you might as well put the +1 in len itself.

Ok, done.

> > -    snprintf(pdf_name, sizeof(pdf_name), "dii%06d", ++email_sequence);
> > +    /* Note; allocating the largest string length to pdf_name */
> > +    int len = strlen(output_directory) + 4 + 6 + 4;
> 
> why 4 before 6 ?

See below.

> > +    pdf_name = (char*)pst_malloc(len + 1);
> > +    snprintf(pdf_name, 3 + 6 + 1, "dii%06d", ++email_sequence);
> 
> it's a 3 here, for "dii".

3 for dii, 6 for the integer and 1 for the terminating \0. What do you
mean?

> >      fprintf(dii_file, "\n@T %s\n", pdf_name);
> > -    snprintf(pdf_name, sizeof(pdf_name), "%s/dii%06d.pdf", output_directory, email_sequence);
> > +    snprintf(pdf_name, len, "%s/dii%06d.pdf", output_directory, email_sequence);

4 for "/dii", 6 for the integer, 4 for ".pdf" and one for the
terminating \0.
 
> Same as above, snprintf should get the total allocated size, otherwise
> it'll get truncated.

Updated patch attached.
diff -ur libpst-0.6.54/configure.in libpst-0.6.54.new/configure.in
--- libpst-0.6.54/configure.in	2012-08-25 18:59:16.000000000 +0200
+++ libpst-0.6.54.new/configure.in	2012-06-27 10:05:54.000000000 +0200
@@ -185,7 +185,7 @@
 fi
 AC_FUNC_STRFTIME
 AC_FUNC_VPRINTF
-AC_CHECK_FUNCS([chdir getcwd memchr memmove memset regcomp strcasecmp strncasecmp strchr strdup strerror strpbrk strrchr strstr strtol])
+AC_CHECK_FUNCS([chdir getcwd memchr memmove memset regcomp strcasecmp strncasecmp strchr strdup strerror strpbrk strrchr strstr strtol get_current_dir_name])
 AM_ICONV
 if test "$am_cv_func_iconv" != "yes"; then
     AC_MSG_ERROR([libpst requires iconv which is missing])
diff -ur libpst-0.6.54/src/libpst.c libpst-0.6.54.new/src/libpst.c
--- libpst-0.6.54/src/libpst.c	2011-12-25 00:19:11.000000000 +0100
+++ libpst-0.6.54.new/src/libpst.c	2012-08-25 19:07:30.000000000 +0200
@@ -289,6 +289,17 @@
 static char*            pst_wide_to_single(char *wt, size_t size);
 
 
+static char *pst_getcwd(void) {
+  char *cwd;
+
+#ifdef HAVE_GET_CURRENT_DIR_NAME
+  cwd = get_current_dir_name();
+#else
+  cwd = pst_malloc(PATH_MAX+1);
+  getcwd(cwd, PATH_MAX+1);
+#endif
+  return cwd;
+}
 
 int pst_open(pst_file *pf, const char *name, const char *charset) {
     int32_t sig;
@@ -361,20 +372,24 @@
 
     DEBUG_RET();
 
-    pf->cwd = pst_malloc(PATH_MAX+1);
-    getcwd(pf->cwd, PATH_MAX+1);
+    pf->cwd = pst_getcwd();
     pf->fname = strdup(name);
     return 0;
 }
 
 
 int  pst_reopen(pst_file *pf) {
-    char cwd[PATH_MAX];
-    if (!getcwd(cwd, PATH_MAX))            return -1;
-    if (chdir(pf->cwd))                    return -1;
-    if (!freopen(pf->fname, "rb", pf->fp)) return -1;
-    if (chdir(cwd))                        return -1;
+    char *cwd;
+    cwd = pst_getcwd();
+    if (cwd == NULL)                       return -1;
+    if (chdir(pf->cwd))                    goto err;
+    if (!freopen(pf->fname, "rb", pf->fp)) goto err;
+    if (chdir(cwd))                        goto err;
+    free(cwd);
     return 0;
+err:
+    free(cwd);
+    return -1;
 }
 
 
diff -ur libpst-0.6.54/src/pst2dii.cpp.in libpst-0.6.54.new/src/pst2dii.cpp.in
--- libpst-0.6.54/src/pst2dii.cpp.in	2011-12-25 00:19:11.000000000 +0100
+++ libpst-0.6.54.new/src/pst2dii.cpp.in	2012-08-26 14:56:07.000000000 +0200
@@ -42,7 +42,7 @@
 char*    font_file = NULL;
 int      bates_color = 0xff0000;    // color of bates header stamp
 int      email_sequence = 0;        // current pdf sequence number
-char     pdf_name[PATH_MAX];        // current pdf file name
+char*    pdf_name = NULL;           // current pdf file name
 FILE*    dii_file = NULL;           // the output dii load file
 pst_file pstfile;                   // the input pst file
 
@@ -344,8 +344,9 @@
 {
     if (png_open) {
         png_open = false;
-        char fn[PATH_MAX];
-        snprintf(fn, sizeof(fn), "page%d.png", ++page_sequence);
+        int len = 4 + 11 + 4 +1;
+        char *fn = (char*)pst_malloc(len);
+        snprintf(fn, len, "page%d.png", ++page_sequence);
         FILE *pngout = fopen(fn, "wb");
         if (pngout) {
             gdImagePng(image, pngout);
@@ -354,6 +355,7 @@
         gdImageDestroy(image); // free memory
         png_names.push_back(fn);
         conversion += string(" ") + fn;
+        free(fn);
     }
 }
 
@@ -366,9 +368,12 @@
     conversion    = string(convert);
     png_names.clear();
     open_png();
-    snprintf(pdf_name, sizeof(pdf_name), "dii%06d", ++email_sequence);
+    /* Note; allocating the largest string length to pdf_name */
+    int len = strlen(output_directory) + 4 + 6 + 4 + 1;
+    pdf_name = (char*)pst_malloc(len);
+    snprintf(pdf_name, 3 + 6 + 1, "dii%06d", ++email_sequence);
     fprintf(dii_file, "\n@T %s\n", pdf_name);
-    snprintf(pdf_name, sizeof(pdf_name), "%s/dii%06d.pdf", output_directory, email_sequence);
+    snprintf(pdf_name, len, "%s/dii%06d.pdf", output_directory, email_sequence);
 }
 
 
@@ -382,6 +387,7 @@
         remove((*i).c_str());
     }
     fprintf(dii_file, "@D %s\n", pdf_name);
+    free(pdf_name);
 }
 
 

Reply to: