Horms wrote:
>>> I think that memory_range, base_memory_range and exclude_range all
>>> need to be initialised to NULL here. Otherwise the checks
>>> in cleanup_memory_ranges() may be bogus.
>>>       
>
> Are globals gauranteed to be NULL? I know that in kernel land they are.
> But I'm not sure about user-space.
>   
I think so :-).

>> +
>> +static void cleanup_memory_ranges()
>> +{
>> +    if (memory_range)
>> +            free(memory_range);
>> +    if (base_memory_range)
>> +            free(base_memory_range);
>> +    if (exclude_range)
>> +            free(exclude_range);
>> +    if (usablemem_rgns.ranges)
>> +            free(usablemem_rgns.ranges);
>> +}
>> +
>> +    return 0;
>> +
>> +err1:
>>     
>       if (memory_range)
>               free(memory_range);
>       if (base_memory_range)
>               free(base_memory_range);
>       if (exclude_range)
>               free(exclude_range);
>
>   
>> +    fprintf(stderr, "memory range structure allocation failure\n");
>> +    cleanup_memory_ranges();
>> +    return -1;
>> +    
>>     
>
> Sorry to be a pain, but I think that you really need something like the
> above under err1. Sorry that I didn't notice that yesterday.
>   
I thought of doing like that as you have mentioned here. But there are 
various exit path's in this file depending on the error, where we should
clean up these memory allocations. Hence i created a seperate function
cleanup_memory_ranges( ) to do that. That way i don't have to 
replicate the above lines all over the exit paths. In the above case
also i am calling cleanup_memory_ranges( ), which eventually is the
same code as you have suggested. Let me kow if that sound ok.

Thanks
-Sachin

>   
>> +}
>> +
>> +/*
>> + * Count the memory@ nodes under /proc/device-tree and populate the
>> + * max_memory_ranges variable. This variable replaces MAX_MEMORY_RANGES
>> + * macro used earlier.
>> + */
>> +static int count_memory_ranges()
>> +{
>> +    char device_tree[256] = "/proc/device-tree/";
>> +    struct dirent *dentry;
>> +    DIR *dir;
>> +
>> +    if ((dir = opendir(device_tree)) == NULL) {
>> +            perror(device_tree);
>> +            return -1;
>> +    }
>> +
>> +    while ((dentry = readdir(dir)) != NULL) {
>> +            if (strncmp(dentry->d_name, "memory@", 7))
>> +                    continue;
>> +            max_memory_ranges++;
>> +    }
>> +    closedir(dir);
>> +
>> +    return 0;
>> +}
>> +
>>  /* Get base memory ranges */
>>  static int get_base_ranges()
>>  {
>> @@ -88,7 +164,7 @@ static int get_base_ranges()
>>                              closedir(dir);
>>                              return -1;
>>                      }
>> -                    if (local_memory_ranges >= MAX_MEMORY_RANGES) {
>> +                    if (local_memory_ranges >= max_memory_ranges) {
>>                              fclose(file);
>>                              break;
>>                      }
>> @@ -447,8 +523,10 @@ int setup_memory_ranges(unsigned long ke
>>       * nodes. Build list of ranges to be excluded from valid memory
>>       */
>>
>> -    get_base_ranges();
>> -    get_devtree_details(kexec_flags);
>> +    if (get_base_ranges())
>> +            goto out;
>> +    if (get_devtree_details(kexec_flags))
>> +            goto out;
>>
>>      for (i = 0; i < nr_exclude_ranges; i++) {
>>              /* If first exclude range does not start with 0, include the
>> @@ -511,12 +589,21 @@ int setup_memory_ranges(unsigned long ke
>>              fprintf(stderr, "setup_memory_ranges memory_range[%d] 
>> start:%lx, end:%lx\n", k, memory_range[k].start, memory_range[k].end);
>>  #endif
>>      return 0;
>> +
>> +out:
>> +    cleanup_memory_ranges();
>> +    return -1;
>>  }
>>
>>  /* Return a list of valid memory ranges */
>>  int get_memory_ranges(struct memory_range **range, int *ranges,
>>                      unsigned long kexec_flags)
>>  {
>> +    if (count_memory_ranges())
>> +            return -1;
>> +    if (alloc_memory_ranges())
>> +            return -1;
>> +
>>      setup_memory_ranges(kexec_flags);
>>      *range = memory_range;
>>      *ranges = nr_memory_ranges;
>> diff -Naurp hormsgittree/kexec/arch/ppc64/kexec-ppc64.h 
>> hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/kexec-ppc64.h
>> --- hormsgittree/kexec/arch/ppc64/kexec-ppc64.h      2006-12-13 
>> 11:25:48.000000000 +0530
>> +++ hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/kexec-ppc64.h        
>> 2006-12-13 11:43:00.000000000 +0530
>> @@ -1,8 +1,6 @@
>>  #ifndef KEXEC_PPC64_H
>>  #define KEXEC_PPC64_H
>>
>> -#define MAX_MEMORY_RANGES 1024 /* TO FIX - needs to be dynamically set */
>> -
>>  #define MAXBYTES 128
>>  #define MAX_LINE 160
>>  #define CORE_TYPE_ELF32 1
>> @@ -17,6 +15,8 @@ void elf_ppc64_usage(void);
>>  void reserve(unsigned long long where, unsigned long long length);
>>
>>  extern unsigned long initrd_base, initrd_size;
>> +extern int max_memory_ranges;
>> +
>>  /* boot block version 2 as defined by the linux kernel */
>>  struct bootblock {
>>      unsigned magic,
>> @@ -39,7 +39,9 @@ struct exclude_range {
>>
>>  typedef struct mem_rgns {
>>          unsigned int size;
>> -        struct exclude_range ranges[MAX_MEMORY_RANGES];
>> +        struct exclude_range *ranges;
>>  } mem_rgns_t;
>>
>> +extern mem_rgns_t usablemem_rgns;
>> +
>>  #endif /* KEXEC_PPC64_H */
>>     
>
>
> --
> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/
>
>   

_______________________________________________
fastboot mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/fastboot

Reply via email to