delorie.com/archives/browse.cgi   search  
Mail Archives: cygwin/2005/07/07/14:06:04

Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com
Date: Thu, 7 Jul 2005 14:05:40 -0400
From: Christopher Faylor <cgf-no-personal-reply-please AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: readshortcut crashes with updated cygwin-1.5.18-1 (cygutils maintainer -- RFA patch enclosed)
Message-ID: <20050707180540.GA15667@trixie.casa.cgf.cx>
Reply-To: cygwin AT cygwin DOT com
References: <7231C15EAC2F164CA6DC326D97493C8BA1C3ED AT exchange35 DOT fed DOT cclrc DOT ac DOT uk>
Mime-Version: 1.0
In-Reply-To: <7231C15EAC2F164CA6DC326D97493C8BA1C3ED@exchange35.fed.cclrc.ac.uk>
User-Agent: Mutt/1.5.8i

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 AT timesys DOT 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/

- Raw text -


  webmaster     delorie software   privacy  
  Copyright © 2019   by DJ Delorie     Updated Jul 2019