Re: parted-1.5.3pre1 fixes

[Andrew Clausen]

On Wed, Jul 18, 2001 at 01:13:49PM +0300, Yura Umanets wrote:
> Hi!
> 
> This is some fixes to parted-1.5.3pre1. See in ChangeLog for detail.

Thanks.  Some of these fixes are broken (and will lead to segfaults,
AFAICT)

> diff -r --unified ./parted-1.5.3-pre1-old/ChangeLog 
> ./parted-1.5.3-pre1/ChangeLog
> --- ./parted-1.5.3-pre1-old/ChangeLog Sat Jul 14 10:46:28 2001
> +++ ./parted-1.5.3-pre1/ChangeLog     Wed Jul 18 12:38:37 2001
> @@ -7,6 +7,10 @@
>  -----------------------------------------------------------------------------
>  1.5.x
>  -----------------------------------------------------------------------------
> +July 18th 2001 - Yury Umanets <address@hidden>
> +* updated swap_probe to avoid segfault.
> +* updated _add_history_unique and _readline to avoid compiller warnings 
> +that cause error when -Werror is turned on.
>  
>  July 13th 2001 - Andrew Clausen <address@hidden>
>  * hacked /usr/share/aclocal/gettext.m4, replacing ac_given_srcdir with
> diff -r --unified 
> ./parted-1.5.3-pre1-old/libparted/fs_linux_swap/linux_swap.c 
> ./parted-1.5.3-pre1/libparted/fs_linux_swap/linux_swap.c
> --- ./parted-1.5.3-pre1-old/libparted/fs_linux_swap/linux_swap.c      Thu Jul 
>  5 12:41:36 2001
> +++ ./parted-1.5.3-pre1/libparted/fs_linux_swap/linux_swap.c  Wed Jul 18 
> 11:39:26 2001
> @@ -128,7 +128,7 @@
>       PedGeometry*    probed_geom;
>       PedSector       length;
>  
> -     if (!swap_open (geom))
> +     if (!(fs = swap_open (geom)))
>               goto error;
>       fs_info = SWAP_SPECIFIC (fs);

This is fine.  However, swap_close() should also be called.  I've
fixed this up.  Thanks :)

> diff -r --unified ./parted-1.5.3-pre1-old/parted/ui.c 
> ./parted-1.5.3-pre1/parted/ui.c
> --- ./parted-1.5.3-pre1-old/parted/ui.c       Tue Jul 10 00:24:44 2001
> +++ ./parted-1.5.3-pre1/parted/ui.c   Wed Jul 18 11:37:04 2001
> @@ -151,8 +151,12 @@
>       HIST_ENTRY*     last_entry = current_history ();
>       if (!strlen (line))
>               return;
> -     if (!last_entry || strcmp (last_entry->line, line))
> -             add_history (line);
> +     if (!last_entry || strcmp (last_entry->line, line)) {
> +             char line_buffer[strlen(line) + 1];
> +             memset(line_buffer, 0, sizeof(line_buffer));
> +             strcpy(line_buffer, line);
> +             add_history (line_buffer);
> +     }       

This is broken.  add_history() doesn't strdup() the buffer.  It
doesn't modify the memory either.  So, your code will pass a
pointer that refers to stack space, but the pointer will referenced
after the stack gets clobbered (after line_buffer falls out of
scope).

The solution is to simply type-cast.

readline() malloc()'s memory, so there's no danger.  Since we don't
kill stuff from history, we don't need to worry about freeing
(but this is doable, if we want to, anyway, via history_remove()).

>  }
>  #endif /* HAVE_LIBREADLINE */
>  
> @@ -175,7 +179,9 @@
>  _readline (const char* prompt, const StrList* possibilities)
>  {
>       char*           line;
> -
> +#ifdef HAVE_LIBREADLINE
> +     char            prompt_buffer[strlen(prompt) + 1];
> +#endif
>       readline_state.possibilities = possibilities;
>       readline_state.cur_pos = NULL;
>       readline_state.in_readline = 1;
> @@ -184,7 +190,9 @@
>               return NULL;
>  
>  #ifdef HAVE_LIBREADLINE
> -     line = readline (prompt);
> +     memset(prompt_buffer, 0, sizeof(prompt_buffer));
> +     strcpy(prompt_buffer, prompt);
> +     line = readline (prompt_buffer);
>       if (line)
>               _add_history_unique (line);
>  #else

Broken for the same reason.

Thanks!
Andrew