This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: readshortcut crashes with updated cygwin-1.5.18-1 (cygutils maintainer -- RFA patch enclosed)


On Thu, Jul 07, 2005 at 05:29:25PM +0100, Adye, TJ (Tim) wrote:
>Hi,
>
>With cygwin-1.5.18-1, the |readshortcut| command crashes on exit if I
>specify a Cygwin-style path name. The crash goes away if I downgrade to
>cygwin 1.5.17-1.

It's a readshortcut double free problem.  As mentioned in another
message, cygwin is using a new version of malloc and it appears to be
more sensitive to these programming errors.

The patch below fixes it.

Chuck, ok to check in?  If so, I'll also get rid of all of the ^Ms in the
readshortcut files.

Btw, in case anyone is interested, I tracked this down by putting a
breakpoint on free, and printing the address being freed.  Then I
cut/pasted that output to a file and sorted the file based on the
address.  This unearthed a few potential candidates for this problem,
one of which turned out to be the the culprit.  I think this is the
fastest I've ever found a double free problem.

So much for my lunch break...

cgf

2005-07-07  Christopher Faylor  <cgf@timesys.com>

	* src/readshortcut.c (readshortcut): Pass opts by reference so that
	changes made to the opts contents are reflected in the caller to
	accommodate freeing of malloced fields.


Index: readshortcut.c
===================================================================
RCS file: /cvs/cygwin-apps/cygutils/src/readshortcut/readshortcut.c,v
retrieving revision 1.1
diff -d -u -p -r1.1 readshortcut.c
--- readshortcut.c	5 Sep 2003 04:07:57 -0000	1.1
+++ readshortcut.c	7 Jul 2005 17:57:28 -0000
@@ -216,7 +216,7 @@ exit:
   return result;
 }
 
-int readshortcut(optvals opts, poptContext optContext) {
+int readshortcut(optvals *opts, poptContext optContext) {
   HRESULT hres;
   IShellLink *shell_link;
   IPersistFile *persist_file;
@@ -227,36 +227,36 @@ int readshortcut(optvals opts, poptConte
   int result = ERR_NONE;  /* the value to return on exit */
   
   /*  Add suffix to link name if necessary */
-  if (strlen (opts.target_fname) > 4) {
-    int tmp = strlen (opts.target_fname) - 4;
-    if (strncmp (opts.target_fname + tmp, ".lnk", 4) != 0) {
-      opts.target_fname = (char *)realloc(opts.target_fname, strlen(opts.target_fname) + 1 + 4);
-      if (opts.target_fname == NULL) {
+  if (strlen (opts->target_fname) > 4) {
+    int tmp = strlen (opts->target_fname) - 4;
+    if (strncmp (opts->target_fname + tmp, ".lnk", 4) != 0) {
+      opts->target_fname = (char *)realloc(opts->target_fname, strlen(opts->target_fname) + 1 + 4);
+      if (opts->target_fname == NULL) {
         fprintf(stderr, "%s: memory allocation error\n", program_name);
         return (ERR_SYS);
       }
-      strcat (opts.target_fname, ".lnk");
+      strcat (opts->target_fname, ".lnk");
     }
   }
   else {
-    opts.target_fname = (char *)realloc(opts.target_fname, strlen(opts.target_fname) + 1 + 4);
-    if (opts.target_fname == NULL) {
+    opts->target_fname = (char *)realloc(opts->target_fname, strlen(opts->target_fname) + 1 + 4);
+    if (opts->target_fname == NULL) {
       fprintf(stderr, "%s: memory allocation error\n", program_name);
       return (ERR_SYS);
     }
-    strcat (opts.target_fname, ".lnk");
+    strcat (opts->target_fname, ".lnk");
   }
 
   /* if there's no colon in the path, it's POSIX and we should convert to win32 */
-  if (strchr (opts.target_fname, ':') == NULL) {
+  if (strchr (opts->target_fname, ':') == NULL) {
     char *strTmpPath = (char*)malloc(MAX_PATH);
     if (strTmpPath == NULL) {
       fprintf(stderr, "%s: memory allocation error\n", program_name);
       return (ERR_SYS);
     }
-    cygwin_conv_to_full_win32_path (opts.target_fname, strTmpPath);
-    free (opts.target_fname);
-    opts.target_fname = strTmpPath;
+    cygwin_conv_to_full_win32_path (opts->target_fname, strTmpPath);
+    free (opts->target_fname);
+    opts->target_fname = strTmpPath;
   }
 
   hres = OleInitialize (NULL);
@@ -280,33 +280,33 @@ int readshortcut(optvals opts, poptConte
     WCHAR wsz[MAX_PATH]; 
  
     /* Ensure that the string is Unicode. */
-    MultiByteToWideChar(CP_ACP, 0, (LPCSTR)opts.target_fname, -1, wsz, MAX_PATH); 
+    MultiByteToWideChar(CP_ACP, 0, (LPCSTR)opts->target_fname, -1, wsz, MAX_PATH); 
 
     /* Load the shortcut.  */
     hres = persist_file->lpVtbl->Load(persist_file, wsz, STGM_READ); 
 
     if (SUCCEEDED(hres)) {
       /* read stuff from the link object and print it to the screen */
-      if (opts.show_all || opts.show_target) {
+      if (opts->show_all || opts->show_target) {
         shell_link->lpVtbl->GetPath(shell_link, strPath, MAX_PATH, NULL, SLGP_RAWPATH);
-        if (opts.show_field_names) { printf("Target: "); }
-        formatPath(strPath, opts.pathType);
+        if (opts->show_field_names) { printf("Target: "); }
+        formatPath(strPath, opts->pathType);
         printf("%s\n", strPath);
       }
-      if (opts.show_all || opts.show_working_dir) {
+      if (opts->show_all || opts->show_working_dir) {
         shell_link->lpVtbl->GetWorkingDirectory(shell_link, strPath, MAX_PATH);
-        if (opts.show_field_names) { printf("Working Directory: "); }
-        formatPath(strPath, opts.pathType);
+        if (opts->show_field_names) { printf("Working Directory: "); }
+        formatPath(strPath, opts->pathType);
         printf("%s\n", strPath);
       }
-      if (opts.show_all || opts.show_args) {
+      if (opts->show_all || opts->show_args) {
         shell_link->lpVtbl->GetArguments(shell_link, strBuff, BUFF_SIZE);
-        if (opts.show_field_names) { printf("Arguments: "); }
+        if (opts->show_field_names) { printf("Arguments: "); }
         printf("%s\n", strBuff);
       }
-      if (opts.show_all || opts.show_showCmd) {
+      if (opts->show_all || opts->show_showCmd) {
         shell_link->lpVtbl->GetShowCmd(shell_link, &iBuff);
-        if (opts.show_field_names) { printf("Show Command: "); }
+        if (opts->show_field_names) { printf("Show Command: "); }
 
         switch (iBuff) {
           case SW_SHOWNORMAL:
@@ -321,26 +321,26 @@ int readshortcut(optvals opts, poptConte
             break;
         }
       }
-      if (opts.show_all || opts.show_icon || opts.show_icon_offset) {
+      if (opts->show_all || opts->show_icon || opts->show_icon_offset) {
         shell_link->lpVtbl->GetIconLocation(shell_link, strPath, MAX_PATH, &iBuff);
-        if (opts.show_all || opts.show_icon) {
-          if (opts.show_field_names) { printf("Icon Library: "); }
-          formatPath(strPath, opts.pathType);
+        if (opts->show_all || opts->show_icon) {
+          if (opts->show_field_names) { printf("Icon Library: "); }
+          formatPath(strPath, opts->pathType);
           printf("%s\n", strPath);
       }
-        if (opts.show_all || opts.show_icon_offset) {
-          if (opts.show_field_names) { printf("Icon Library Offset: "); }
+        if (opts->show_all || opts->show_icon_offset) {
+          if (opts->show_field_names) { printf("Icon Library Offset: "); }
           printf("%d\n", iBuff);
         }
       }
-      if (opts.show_all || opts.show_desc) {
+      if (opts->show_all || opts->show_desc) {
         shell_link->lpVtbl->GetDescription(shell_link, strBuff, BUFF_SIZE);
-        if (opts.show_field_names) { printf("Description: "); }
+        if (opts->show_field_names) { printf("Description: "); }
         printf("%s\n", strBuff);
       }
     }
     else {
-      fprintf (stderr, "%s: Load failed on %s\n", program_name, opts.target_fname);
+      fprintf (stderr, "%s: Load failed on %s\n", program_name, opts->target_fname);
       result = ERR_WIN;
     }
 

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]