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)


Thanks! The cygutils-1.2.9-1 update fixes the problem.

Tim.

> -----Original Message-----
> From: Christopher Faylor
> Sent: 07 July 2005 19:06
> To: cygwin@cygwin.com
> Subject: 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]