public inbox for bzip2-devel@sourceware.org
 help / color / mirror / Atom feed
From: Carlo Bramini <carlo.bramix@libero.it>
To: "bzip2-devel@sourceware.org" <bzip2-devel@sourceware.org>
Subject: Re: Declare bzerrorstrings[] to be a const object.
Date: Wed, 27 Apr 2022 11:21:45 +0200 (CEST)	[thread overview]
Message-ID: <800388119.1021596.1651051305973@mail1.libero.it> (raw)
In-Reply-To: <YmhJZdpiNFhxeM8r@wildebeest.org>

Hello,
I compiled BZip2 for an embedded system, to be used in an homebrew project.
The platform is an ARM Cortex M4 microcontroller which provides some internal memories, like all other devices with similar purpose: 
* a FLASH memory, which is usually bigger and it stores the code and constant data of a firmware.
* a RAM memory, which is tipically much smaller than the FLASH and it is used for the stack and all data that can change their value during the execution of the program.

Since the RAM memory is a limited resource, one of the tasks of the developer is to move all data that won't change their value into the FLASH, not only for security reasons but because it is needed to do so, for maximizing the amount of available RAM.

To do so, the developer usually examines a map file to see if some memory could be optimized, especially when using code written by others.

So, I gave a look to the map file and I have seen that something could be improved in this way.

In this specific case, the bzerrorstrings[] is actually made by two distinct logical objects:
1) the bzerrorstrings[], which holds some pointers to strings.
2) the strings pointed by the bzerrorstrings[].

Actually, you can decide which of these items is a const object: the strings, the bzerrorstrings[] table or both.

The attached patch just adds another const attribute for having both items into the read-only area, which seems to be the expected behaviour of that piece of code.

It is true that this fix won't improve the memory usage on a system like a PC, however it could be also useful since an exception could be raised if something will accidentally write into a memory expected to be read-only.

By looking my map file, I have to say that there are also other things that may be declared as const:

blocksort.c:
- static Int32 incs[14]
bzip2.c
- const Char* zSuffix[BZ_N_SUFFIX_PAIRS]
- const Char* unzSuffix[BZ_N_SUFFIX_PAIRS]
crctable.c
- UInt32 BZ2_crc32Table[256]
randtable.c
- Int32 BZ2_rNums[512]

but unfortunately I'm unsure about the code, so the bzerrorstrings[] table was the easiest one for me to understand that it was a safe change.

I hope that you will find this report useful.
Sincerely,

Carlo Bramini.


> Il 26/04/2022 21:35 Mark Wielaard <mark@klomp.org> ha scritto:
> 
>  
> Hi,
> 
> On Tue, Apr 26, 2022 at 04:33:22PM +0200, Carlo Bramini via Bzip2-devel wrote:
> > by looking the content of the map file, I discovered that bzerrorstrings[] is not const.
> 
> Which map file is this?
> 
> > So, I would like to suggest the tiny change into the attached patch, for declaring both bzerrorstrings[] and the addressed strings as const.
> > The patch has been created from the latest sources into the repository with GIT.
> 
> Although the patch seems fine I must admit that I don't fully
> understand why it is necessary. Isn't an array (name) not always
> constant? You cannot assign anything to an array name since it is a
> contant pointer to the first element. Or is that not what this "const"
> is about?
> 
> Thanks,
> 
> Mark

      reply	other threads:[~2022-04-27  9:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-26 14:33 Carlo Bramini
2022-04-26 19:35 ` Mark Wielaard
2022-04-27  9:21   ` Carlo Bramini [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=800388119.1021596.1651051305973@mail1.libero.it \
    --to=carlo.bramix@libero.it \
    --cc=bzip2-devel@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).