Home Original page

[Patch, tentative, AVR] Displaying per-device memory usage info

Andrew Burgess andrew.burgess@embecosm.com
Mon Nov 24 18:04:00 GMT 2014 
I can't give you commit approval.  I spotted a few minor coding
standard issues while taking a look at this patch, comments inline
below.

Thanks,
Andrew

* Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> [2014-11-17 18:47:56 +0530]:

> diff --git binutils/od-elf32_avr.c binutils/od-elf32_avr.c
> new file mode 100644
> index 0000000..f74d602
> --- /dev/null
> +++ binutils/od-elf32_avr.c
> @@ -0,0 +1,243 @@
> +/* od-avrelf.c -- dump information about an xcoff object file.

That header line seems wrong.

> +static char * elf32_avr_get_note_section_contents (bfd *abfd, bfd_size_type *size)

Should have newline after "static char *".

> +
> +static char* elf32_avr_get_note_desc (bfd *abfd, char *contents,
> +        bfd_size_type size)

Should be "static char *" then newline.

> +static void
> +elf32_avr_get_device_info (bfd *abfd, char *description,
> +        deviceinfo *device)
> +{
> +  if (description == NULL)
> +    return;
> +
> +  const bfd_size_type memory_sizes = 6;
> +
> +  memcpy (device, description, memory_sizes * sizeof(uint32_t));

Whitespace after sizeof.

> +  device->name = NULL;
> +
> +  uint32_t *stroffset_table = ((uint32_t *)description) + memory_sizes;

Should have whitespace after cast.

> +  bfd_size_type stroffset_table_size = bfd_get_32 (abfd, stroffset_table);
> +  char *str_table = ((char *)stroffset_table) + stroffset_table_size;

Whitespace after cast.

> +static void
> +elf32_avr_dump (bfd *abfd)
> +{
> +  char *description = NULL;
> +  bfd_size_type note_section_size = 0;
> +
> +  deviceinfo device = {0};
> +
> +  bfd_size_type data_usage = 0;
> +  bfd_size_type text_usage = 0;
> +  bfd_size_type eeprom_usage = 0;
> +
> +  if (!options[OPT_MEMUSAGE].selected)
> +    return;

In order to allow for future code growth it might be nice to move the
core of this function out, into elf32_avr_dump_mem_usage, then write:

    if (options[OPT_MEMUSAGE].selected)
    	elf32_avr_dump_mem_usage (abfd);

this way if/when more options are added the code changes required are
reduced. What do you think?

> +
> +  device.name = "Unknown";
> +
> +  char *contents = elf32_avr_get_note_section_contents (abfd,
> +    &note_section_size);
> +
> +  if (contents != NULL)
> +    {
> +      description = elf32_avr_get_note_desc (abfd, contents, note_section_size);
> +      elf32_avr_get_device_info (abfd, description, &device);
> +    }
> +
> +  elf32_avr_get_memory_usage (abfd, &text_usage, &data_usage,
> +     &eeprom_usage);
> +
> +  printf ("AVR Memory Usage\n"
> +          "----------------\n"
> +          "Device: %s\n\n", device.name);
> +
> +  /* Text size */
> +  printf ("Program:%8ld bytes", text_usage);
> +  if (device.flash_size > 0)
> +    printf (" (%2.1f%% Full)", ((float)text_usage / device.flash_size) * 100);

Whitespace after cast.

> +
> +  printf ("\n(.text + .data + .bootloader)\n\n");
> +
> +  /* Data size */
> +  printf ("Data:   %8ld bytes", data_usage);
> +  if (device.ram_size > 0)
> +  {
> +    printf (" (%2.1f%% Full)", ((float)data_usage / device.ram_size) * 100);

Whitespace after cast.

> +  }
> +  printf ("\n(.data + .bss + .noinit)\n\n");
> +
> +  /* EEPROM size */
> +  if (device.eeprom_size > 0)
> +    {
> +      printf ("EEPROM: %8ld bytes", eeprom_usage);
> +      printf (" (%2.1f%% Full)", ((float)eeprom_usage / device.eeprom_size) * 100);

Whitespace after cast.

> +      printf ("\n(.eeprom)\n\n");
> +    }
> +
> +  if (contents != NULL)
> +    free (contents);
> +}
More information about the Binutils mailing list