Struct mit String im Flash Zugriff

[Christian (Gast)]

Hallo,

ich habe ein Veständisproblem mit den Zugriff auf Strings welche im 
Flash liegen und der Zeiger davon in einem Struct was auch im Flash 
liegt.

//////////////////////////////////////////////////////////////////////////  
typedef struct{
  const char __flash * const cmdString;
  const uint8_t cmd;
  uint8_t (*prog_fnc)(uint8_t*);      /* prog function */
}t_CommandStruct;


//////////////////////////////////////////////////////////////////////////

static const char __flash strPort[] = "PORT";
static const char __flash strVersion[] = "VERSION";


//////////////////////////////////////////////////////////////////////////
static uint8_t cmd_port(uint8_t* ptr);
static uint8_t cmd_version(uint8_t* ptr);

//////////////////////////////////////////////////////////////////////////

uint8_t incom_buf[25];

//////////////////////////////////////////////////////////////////////////

static const t_CommandStruct __flash CommandStruct[] =
{
  { strPort, 1, cmd_port},
  { strVersion, 1, cmd_version},
  { FSTR ("\0"), 0, NULL},
};


static uint8_t intr_msg(void)
{
  uint8_t i=0;

  do
  {
    if(CommandStruct[i].cmdString[0] == '\0') {
      break;
    }
      

    printf_P(CommandStruct[i].cmdString);
    if(strcmp_P( (const char*)incom_buf, CommandStruct[i].cmdString/*, strlen_P(CommandStruct[i].cmdString)*/) == 0) {
      CommandStruct[i].prog_fnc(incom_buf);  
    }
    i++;
       
  }while(1);
        
        return 1;
}

Der Code funktioniert, nur was mich stört das folgende Meldung bekomme:
Warnung  conversion from address space '__flash' to address space 
'generic' [-Waddr-space-convert]

Ist das normal, sollte man die Warnung deaktivieren?



Grüße
Christian

[Karl H. (kbuchegg)]

Christian schrieb:

> Ist das normal, sollte man die Warnung deaktivieren?

Nein, auf keinen Fall.
An welcher Stelle kommt denn die Warnung?

[Christian (Gast)]

Ah sorry, diese Angabe hab ich natürlich vergessen.
Die Meldung bezieht sich auf printf_P und strcmp_P.

[Jörg W. (dl8dtl) (Moderator)]

Ich denke, dass du sie an dieser Stelle wirklich ignorieren kannst.

Im Prinzip würde man diese Funktionen wohl heutzutage gleich auf dem
__flash address space aufsetzen.  Wenn man das aber macht, compiliert
der historisch gewachsene Code nicht mehr.  Hmm, naja, vielleicht
compiliert er ja noch, bringt aber dann eine Warnung (halt umgekehrt)?
Braucht dann eben nur einen hinreichend neuen Compiler (damit __flash
unterstützt wird), aber das sollte ja kein Problem mehr sein.

An sich wäre das eine API-Änderung, damit müsste man das Resultat
eigentlich dann avr-libc 2.0 nennen.  Bin ich aber nicht völlig
abgeneigt, das zu tun. ;)

[Eric B. (beric)]

Christian schrieb:
> FSTR ("\0")

Wat is dat?

Willst du hier einen NULL-Pointer? Dann schreib NULL oder 0!
Willst du einen leeren String? Dann schreib "" (oder halt FSTR(""))

[Johann L. (gjlayde)]

Christian schrieb:
> Der Code funktioniert, nur was mich stört das folgende Meldung bekomme:
> Warnung  conversion from address space '__flash' to address space
> 'generic' [-Waddr-space-convert]
>
> Ist das normal, sollte man die Warnung deaktivieren?

Ja, die Warnung würd ich nur sporadisch verwenden.

Die Warnung hab ich seinerzeit zusammen mit Address-Space (AS) __flash 
et al. eingeführt um bei Portierung von PROGMEM + pgm_xxx --> __flash 
behilflich zu sein.

Problem ist jedoch, dass z.B. die Lib-Funktionen alle ohne 
AS-Qualifier geschrieben sind und Literale wie in PSTR keine 
AS-Qualifier haben (dürfen).  Daher gibt es auch kein AS-Äquivalent zu 
PSTR.

Somit ist es kaum möglich, Code zu schreiben, der AS verwenden und diese 
Warnung nicht produziert.  Grund ist auch, dass es in GCC keine 
einfache Möglichkeit gibt, zwischen expliziten Casts und einem 
impliziten Casts zu unterscheiden.

Solche Fragen sind übrigens einfacher zu beantworten, wenn der gezeigte 
"Code" compilierbar ist: Es fehlen Deklarationen von FSTR, uint8_t, 
strcmp_P, ...

[Christian (Gast)]

@Eric und Johann:
#define FSTR(X) ((const __flash char[]) { X } )

@Jörg:
Danke, vielleicht sollte man wirklich mal einen Schnitt machen. 
Irgendwie sorgen solche Meldungen schon für Verwirrungen, noch dazu wenn 
man im Netz nicht wirklich Infos findet.
Zumindest beruhigt es mich, das mit meinen Struct nicht dran schuld ist 
:)

[Jörg W. (dl8dtl) (Moderator)]

Eric B. schrieb:
> Willst du hier einen NULL-Pointer?

Der hätte an dieser Stelle übrigens deutlich mehr Sinn, und dann
die Auswertung mit

    if(CommandStruct[i].cmdString == NULL) {
      break;
    }

Das spart eine Dereferenzierung aus dem Flash.

[Eric B. (beric)]

Christian schrieb:
> @Eric und Johann:
> #define FSTR(X) ((const __flash char[]) { X } )

Ich habe mich eher auf das "\0" bezogen. Das macht gar kein Sinn.

[Karl H. (kbuchegg)]

Jörg W. schrieb:

> An sich wäre das eine API-Änderung, damit müsste man das Resultat
> eigentlich dann avr-libc 2.0 nennen.  Bin ich aber nicht völlig
> abgeneigt, das zu tun. ;)

Wäre vernünftig.
Es gibt ja auch noch die Funktion strcmp_PF. Ursprünglich dachte ich, 
die wären die richtigen, bis ich bemerkte, dass das F für 'far' steht 
und nicht für 'flash'

Gut wäre es schon, denn wenn bei einem normalen strcmp die Warnung 
kommt, dann ist sie schwerwiegend, bei einem strcmp_P jedoch nicht. 
Ausser die Reihenfolge der Argumente ist genau falsch herum. Dann ist 
sie wieder schwerwiegend. D.h. hier verliert man kräftig an 
Typsicherheit, wenn man die Warnung einfach ignoriert. Und auf diese 
Warnung würde ich ehrlich gesagt auf keinen Fall verzichten wollen.

Übrigens krieg ich bei meinem mit dem Studio 6.2 mitgeliefertem Compielr 
keine Warnung.

[Johann L. (gjlayde)]

Jörg W. schrieb:
> Im Prinzip würde man diese Funktionen wohl heutzutage gleich auf dem
> __flash address space aufsetzen.  Wenn man das aber macht, compiliert
> der historisch gewachsene Code nicht mehr.  Hmm, naja, vielleicht
> compiliert er ja noch, bringt aber dann eine Warnung (halt umgekehrt)?
> Braucht dann eben nur einen hinreichend neuen Compiler (damit __flash
> unterstützt wird), aber das sollte ja kein Problem mehr sein.
>
> An sich wäre das eine API-Änderung, damit müsste man das Resultat
> eigentlich dann avr-libc 2.0 nennen.  Bin ich aber nicht völlig
> abgeneigt, das zu tun. ;)

IMO bringt es keinen nennenswerten Mehrwert, zumal die alten Prototypen 
weiter bestehen werden und die Funktionen den gleichen Code erzeugen, 
d.h. lediglich Aliases auf bestehenden Code wären.

Und willst du Versionen für die bis zu 8 Spaces (generic, memx, flash, 
flash1 ... flash5) implementieren?

[Jörg W. (dl8dtl) (Moderator)]

Johann L. schrieb:
> Und willst du Versionen für die bis zu 8 Spaces (generic, memx, flash,
> flash1 ... flash5) implementieren?

Nö, ich wollte keine neuen Funktionen implementieren.  Die bisherigen
*_P()-Deklaration dagegen haben ja rein gar nichts, was einen darauf
hinweisen würde, dass man da einen falschen Zeiger übergibt:

  const char fmt[] = "%d";

  printf_P(fmt);

Dass es kein Äquivalent zu PSTR() gibt, ist allerdings in der Tat 
schade.

[Johann L. (gjlayde)]

Jörg W. schrieb:
> Johann L. schrieb:
>> Und willst du Versionen für die bis zu 8 Spaces (generic, memx, flash,
>> flash1 ... flash5) implementieren?
>
> Nö, ich wollte keine neuen Funktionen implementieren.  Die bisherigen
> *_P()-Deklaration dagegen haben ja rein gar nichts, was einen darauf
> hinweisen würde, dass man da einen falschen Zeiger übergibt:
>
>

const char fmt[] = "%d";
> 
>   printf_P(fmt);

Wird aber trotzdem recht ätzend, das müsste ja auch für C99 etc. 
funktionieren, die AS sind aber nur in GNU-C verfügbar.

Außerdem ist das löd mit altem, funktionierendem Code der PROGMEM etc. 
verwendet.  Generell find ich blöd, wenn funktionierender Code Probleme 
bekommt, nur weil neuer Code neue Gimmicks verwendet / verwenden könnte.


> Dass es kein Äquivalent zu PSTR() gibt, ist allerdings in der Tat
> schade.

hmmm, folgendes scheint zu gehen, ganz analog zu PSTR:

#define XSTR(X) ({ static const __flash char __s[] = X; __s;})

extern int strcmp_F (const char*, const __flash char*);

int afoo (const char *s)
{
    return strcmp_F (s, XSTR ("Hallo"));
}

char cfoo (int i)
{
    return XSTR ("Hallo")[i];
}

const __flash char* pfoo (int i)
{
    return & XSTR ("Hallo")[i];
}

[Jörg W. (dl8dtl) (Moderator)]

Johann L. schrieb:

> Wird aber trotzdem recht ätzend, das müsste ja auch für C99 etc.
> funktionieren, die AS sind aber nur in GNU-C verfügbar.

Eigentlich auch schade.  Ich weiß, AS ist kein expliziter Bestandteil
des Standards, aber zumindest ein Standardvorschlag.

Man kann ja mit _FLASH intern arbeiten und dieses nur dann auf
__flash setzen, wenn klar ist, dass es funktionieren wird.  Dann
kann man auch noch einen Kombatibilitätsknopf für alten Code
einführen.

> Außerdem ist das löd mit altem, funktionierendem Code der PROGMEM etc.
> verwendet.

Wobei wir ja mit den früheren Definitionen für prog_char etc. ohnehin
so hinreichend in die braune Masse gegriffen haben, dass es darauf
nun auch nicht mehr ankommt. ;-)

>> Dass es kein Äquivalent zu PSTR() gibt, ist allerdings in der Tat
>> schade.
>
> hmmm, folgendes scheint zu gehen, ganz analog zu PSTR:

Danke!