Re: [PATCH] AFS fixes and improvements

[Vladimir 'phcoder' Serbinenko]

> I prefer not to fix minor memory leaks by huge patches.  I have a patch,
> I just wanted to look at the possibility to use grub_kzalloc() and
> allocating data as part of the partition.
Well nested partition patch changes the way how partitions are handled
and fix for memory leak probably wouldn't stay long and will just
cause an unnecessary rediffing. I prefer to commit a big patch and
then look if memory leak remains and if it's the case fix the memory
leak
>
> Anyway, here is the preliminary version.
>
> Fix memory leak in grub_disk_close()
>
>        * kern/disk.c (grub_disk_close): Free disk->partition->data.
>        * partmap/acorn.c (acorn_partition_map_probe): Set data to NULL.
>        * partmap/amiga.c (amiga_partition_map_iterate): Likewise.
>        * partmap/apple.c (apple_partition_map_iterate): Likewise.
>        * partmap/gpt.c (gpt_partition_map_iterate): Likewise.  We never
>        use it.
Wrong. It's used in grub-setup.c. I already stepped into this pitfall
myself. I'll check if  I actually sent the last version (same as in my
repo) of nested partition patch
> ---
>  kern/disk.c     |    3 +++
>  partmap/acorn.c |    1 +
>  partmap/amiga.c |    1 +
>  partmap/apple.c |    1 +
>  partmap/gpt.c   |    2 +-
>  5 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/kern/disk.c b/kern/disk.c
> index e463626..e6d6ab6 100644
> --- a/kern/disk.c
> +++ b/kern/disk.c
> @@ -338,6 +338,9 @@ grub_disk_close (grub_disk_t disk)
>   /* Reset the timer.  */
>   grub_last_time = grub_get_time_ms ();
>
> +  if (disk->partition)
> +    grub_free (disk->partition->data);
> +
>   grub_free (disk->partition);
>   grub_free ((void *) disk->name);
>   grub_free (disk);
> diff --git a/partmap/acorn.c b/partmap/acorn.c
> index 42fd61f..ef12043 100644
> --- a/partmap/acorn.c
> +++ b/partmap/acorn.c
> @@ -164,6 +164,7 @@ acorn_partition_map_probe (grub_disk_t disk, const char 
> *str)
>   p->len = map[partnum].size;
>   p->offset = 6;
>   p->index = partnum;
> +  p->data = NULL;
>   return p;
>
>  fail:
> diff --git a/partmap/amiga.c b/partmap/amiga.c
> index ffb807f..91289d4 100644
> --- a/partmap/amiga.c
> +++ b/partmap/amiga.c
> @@ -124,6 +124,7 @@ amiga_partition_map_iterate (grub_disk_t disk,
>
>       part.offset = (grub_off_t) next * 512;
>       part.index = partno;
> +      part.data = NULL;
>       part.partmap = &grub_amiga_partition_map;
>
>       if (hook (disk, &part))
> diff --git a/partmap/apple.c b/partmap/apple.c
> index fce2f2c..6d673ad 100644
> --- a/partmap/apple.c
> +++ b/partmap/apple.c
> @@ -146,6 +146,7 @@ apple_partition_map_iterate (grub_disk_t disk,
>       part.len = grub_be_to_cpu32 (apart.blockcnt);
>       part.offset = pos;
>       part.index = partno;
> +      part.data = NULL;
>
>       grub_dprintf ("partition",
>                    "partition %d: name %s, type %s, start 0x%x, len 0x%x\n",
> diff --git a/partmap/gpt.c b/partmap/gpt.c
> index d646d41..35a979d 100644
> --- a/partmap/gpt.c
> +++ b/partmap/gpt.c
> @@ -92,7 +92,7 @@ gpt_partition_map_iterate (grub_disk_t disk,
>          part.offset = entries;
>          part.index = i;
>          part.partmap = &grub_gpt_partition_map;
> -         part.data = &entry;
> +         part.data = NULL;
>
>          grub_dprintf ("gpt", "GPT entry %d: start=%lld, length=%lld\n", i,
>                        (unsigned long long) part.start,
>
>
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git