Better method for PROGMEM data access with multiple indirection?

Question

-edit-
This is a simplified recreation of personal learning project I have ongoing. My issue is in accessing data in flash, the path to which includes multiple layers of indirection. When the data was in RAM I found accessing it easy and all worked as expected.

In flash I have created a struct mylist, aa in the example, containing a pointer to an char* array (item_list[ ])(in flash) pointing to char[ ] (in flash).

Problem: (refer to example) Given a pointer to struct aa (pml) and an index into item_list[] (ndx), how to correctly retrieve the string from flash. My example illustrates the only method I can find that works. It seems kludgy at best, in particular needing to twice read pointers from flash as well as calculating the array item offset. Have I missed a proper cast or perhaps become confused about 32 vs 16 bit pointers? Or, as strange as it seems, is this the correct and/or best approach?

All help and constructive criticism will be appreciated.

Code:

#include "Arduino.h"
#include <avr/pgmspace.h>

const char x[] PROGMEM = {"Item x"};
const char y[] PROGMEM = {"Item y"};
const char* const item_list[] PROGMEM = {x, y};

struct mylist {
  uint8_t nitems;
  const char** const list;
};
const mylist aa PROGMEM = {2,(const char** const)item_list};

const mylist* pml = &aa;

void setup() {
  Serial.begin(9600);

  char cbuf[11] = {"__________"};

  // two line with declared intermediate pointer
  uint8_t ndx = 0;
  const char* pitem = (const char*)pgm_read_ptr_near(&pml->list) + (ndx * sizeof(const char*));
  strcpy_P(cbuf, (const char*)pgm_read_ptr_near(pitem));
  Serial.print("item[0]: /");
  Serial.print(cbuf);
  Serial.println("/");

  // one line without declared intermediate pointer
  ndx++;
  strcpy_P(cbuf, (const char*)pgm_read_ptr_near(pgm_read_ptr_near(&pml->list)
                                                + (ndx * sizeof(const char*))));
  Serial.print("item[1]: /");
  Serial.print(cbuf);
  Serial.println("/");
}

void loop() {
  // put your main code here, to run repeatedly:

}

Output:

item[0]: /Item x/
item[1]: /Item y/

-edit-

After many more hours trying many more variations, I feel the approach shown in the following example is workable for me. I have chosen to keep the extra level of indirection to maintain all the flash data for a particular Field in one struct ‘package’. @NickGammon and his excellent examples greatly helped my understanding and comfort level with pointers in and to flash.

The listing is meant to give context to my point in question and is summarized in the method _copyitem( ) (hope the underscore is ok):

void Field::_copyitem(uint8_t ndx){
  PGM_VOID_P plist = pgm_read_ptr(&_fp->list);
  PGM_P pitem = (PGM_P)pgm_read_ptr(plist + ndx * sizeof(char*));
  strlcpy_P(_value, pitem, getLength());
}

This listing recreates a very small subset of the overall codebase, but the one part that has stopped my progress.

#include "Arduino.h"
#include <avr/pgmspace.h>

struct FieldDefinition {
  uint8_t nitems;
  const char** list;
  uint8_t row;
  uint8_t column;
  uint8_t length;
  // etc, etc
};

class Field {
public:
  Field(const FieldDefinition*  /* other parameters */);
  ~Field();  // -edit-
  char * selectItem(uint8_t);
  uint8_t getLength();
  char * selectItem(uint8_t);
  uint8_t getLength();
private:
  const FieldDefinition * _fp;
  char * _value;
  uint8_t _ndx;
  void _copyitem(uint8_t);
};

Field::Field(const FieldDefinition* fp ) : _fp(fp) {
  _ndx = 0;
  char * _value = new char[getLength()];
 _copyitem(_ndx);
};
Field::~Field(){  // -edit-
  delete _value;
};
char * Field::selectItem(uint8_t ndx){
  _ndx = ndx;
  _copyitem(_ndx);
  // display item
  // do more here . . .
 return _value;
}
uint8_t Field::getLength(){
  return pgm_read_byte(&_fp->length);
}
void Field::_copyitem(uint8_t ndx){
  PGM_VOID_P plist = pgm_read_ptr(&_fp->list);
  PGM_P pitem = (PGM_P)pgm_read_ptr(plist + ndx * sizeof(char*));
  strlcpy_P(_value, pitem, getLength());
}

// Setup flash data
// selection list
const char f1_0[] PROGMEM = {"Item 0"};
const char f1_1[] PROGMEM = {"Item 1"};
const char f1_2[] PROGMEM = {"Item 2"};
const char* const f1_list[] PROGMEM = {f1_0, f1_1, f1_2};
// create in-flash struct
const FieldDefinition f1 PROGMEM = {4,(const char**)f1_list,1,1,10};
// global instance and pointer to Field
Field * s1f1 = new Field(&f1);

void setup() {
  Serial.begin(115200);
  Serial.println();

  Serial.println();
  Serial.print("Field Length: ");
  Serial.println(s1f1->getLength());
  Serial.println();
  for (uint8_t i = 0 ; i < 3; i++) {
    Serial.print("Selection at index ");
    Serial.print(i); Serial.print(" : ");
    Serial.println(s1f1->selectItem(i));
  }
}

void loop() {
  // put your main code here, to run repeatedly:

}

Output

Field Length: 10

Selection at index 0 : Item 0
Selection at index 1 : Item 1
Selection at index 2 : Item 2

I could very wrong (as previously demonstrated) so comments please.

-edit-

Point taken and completely understood. Destructor added for completeness. In my actual application, multiple fields are constructed by a screen class when a screen is displayed, user interaction accepted, configuration data stored in EEPROM (my next endeavor), then destroyed on screen exit. Selecting from a list is one method available to populate the field value. The whole interface consists of multiple fields, for multiple screens, driven by an underlying state engine, stepped during each ‘loop’ to minimize impact. It functions as expected in an Xcode simulation, and has been a wonderful learning experience moving it to Arduino. I thought all this was non sequitur to my question and regret any confusion I may have caused.

To restate my issue: how to get from

// ram based
strcpy(value, _fp->list[ndx]);

to

// flash based
PGM_VOID_P plist = pgm_read_ptr(&_fp->list);
PGM_P pitem = (PGM_P)pgm_read_ptr(plist + ndx * sizeof(char*));
strcpy_P(value, pitem);  

using best/correct form with multiple layers of indirection as posted. Is it possible I have missed a proper cast or other mechanism that would allow the compiler to handle the ‘pointer magic’? It appears I have not. @NickGammon I greatly appreciate your input.

Answer 1

const char x[] PROGMEM = {"Item x"};
const char y[] PROGMEM = {"Item y"};
const char* const item_list[] PROGMEM = {x, y};

struct mylist {
  uint8_t nitems;
  const char** const list;
};
const mylist aa PROGMEM = {2,(const char** const)item_list};

It seems to me this is way too wordy for what appears to be trying to store a list of a couple of strings.

---

First:

const char* const item_list[] PROGMEM = {x, y};

You have a array of pointers to strings. OK.

---

Now:

struct mylist {
  uint8_t nitems;
  const char** const list;
};
const mylist aa PROGMEM = {2,(const char** const)item_list};

Now you have a list of an array? This is where I am confused.

---

Compare to an adaptation of the code on Putting constant data into program memory (PROGMEM):

const int NUMBER_OF_ELEMENTS = 2;

const char Message0000 [] PROGMEM = "Item X"; 
const char Message0001 [] PROGMEM = "Item Y"; 

const char * const messages[NUMBER_OF_ELEMENTS] PROGMEM = 
   { 
   Message0000, 
   Message0001, 
   };

void setup ()
  {
  Serial.begin (115200);
  Serial.println ();

  for (int i = 0; i < NUMBER_OF_ELEMENTS; i++)
    {
    char * ptr = (char *) pgm_read_word (&messages [i]);
    char buffer [80]; // must be large enough!
    strcpy_P (buffer, ptr);
    Serial.println (buffer);
    }   // end of for loop

  }  // end of setup

void loop () { } 

---

Edited to comment on new code

You have a major problem here:

Field::Field(const FieldDefinition* fp ) : _fp(fp) {
  _ndx = 0;
  char * _value = new char[getLength()];
 _copyitem(_ndx);
};
char * Field::selectItem(uint8_t ndx){
  _ndx = ndx;
  _copyitem(_ndx);
  // display item
  // do more here . . .
 return _value;
}
uint8_t Field::getLength(){
  return pgm_read_byte(&_fp->length);
}
void Field::_copyitem(uint8_t ndx){
  PGM_VOID_P plist = pgm_read_ptr(&_fp->list);
  PGM_P pitem = (PGM_P)pgm_read_ptr(plist + ndx * sizeof(char*));
  strlcpy_P(_value, pitem, getLength());
}

What I gather you are doing here is, to get the string for an item (for printing) doing a new (which allocates memory), copying from PROGMEM into that memory, displaying that item, and then ignoring the allocated memory. Well, you can't do that. You will run out of RAM very quickly.

The code doesn't even have the word delete in it. Every new should have a corresponding delete. It's a fundamental principle.

It looks too complex still. Make a function that just displays the item (menu or whatever it is) without actually making a copy of it. Certainly don't allocate memory and not free it.