[request for comment] strlen_P() enhancement

26 posts / 0 new
Last post
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Proposed enhancement to strlen_P():

#define my_strlen_P(s)                         \
  __builtin_choose_expr(                       \
    __builtin_constant_p(__builtin_strlen(s)), \
    __builtin_strlen(s),                       \
    strlen_P(s)                                \
  )

This should avoid calling the strlen_P function if the string size is known to the compiler beforehand (i.e. if the parameter to strlen_P is not a changing pointer). This is in fact how strlen() works anyway (through a mechanism in __builtin_strlen() hidden to the user).

Of course, the real change would involve also to rename the current library function strlen_P() to say __strlen_P() and then this macro would be part of pgmspace.h, calling __strlen_P() in cases where appropriate.

Comment, please.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Do you want to know the strlen() of the anonymous string or the sizeof() the anonymous string?

Surely the strlen() will just be sizeof() - 1. And you can use a macro for it.

Just exactly what are you trying to do?
If it is anonymous, you would need a new instance for each reference. e.g.

printf_P(PSTR("anonymous format"));
len = sizeof(PSTR("anonymous format")) - 1;
len = strlen_P(PSTR("anonymous format"));

if the string has an identifier:

char identifier[] PROGMEM = "identified string";
len = sizeof(identifier) - 1;
len = strlen_P(identifier);

Untested. But the non-anonymous version looks preferable. And should be portable across compilers. (just by using appropriate storage types)

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

David,

PSTR("string") is seen as a pointer, so (in avr-gcc) sizeof(PSTR("string")) == 2. That's why sizeof() is not an universal operator to be recommended for this case. C, as a true bastard "middle-level" language, has implemented strings through a kludge; that's why it's better to use the dedicated functions from string.h (and the equivalents from pgmspace.h).

strlen_P(PSTR("anything")) results if function call, which actually scans through the string in FLASH; but as the string in FLASH can't change, this is a known constant compile-time. That is what I seek to remedy. strlen() works in this way already; I think it makes sense to fix strlen_P() to work in the same way.

Portability is no issue in case of strlen_P(), as it is non-standard avr-libc -specific function.

I am not discussing alternative ways of finding a string's length from the user's point of view; I am suggesting an enhancement for an existing library function.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jan,

The following source

char identifier[] PROGMEM = "identified string";
size = sizeof(identifier) - 1;

produces this:

.global	identifier
	.section	.progmem.data,"a",@progbits
	.type	identifier, @object
	.size	identifier, 18
identifier:
	.string	"identified string"
.global	size
	.data
	.type	size, @object
	.size	size, 2
size:
	.word	17

Admittedly 'size' is a variable in the DATA segment. It is initialised to the correct value.

I see no problem with using a macro for the operation. The non-anonymous string is declared as an array not as a pointer.

I would agree that trying to get the length of an anonymous pointer to an anonymous string requires strlen_P(). But presumably you only want a single anonymous string, or else you are just wasting FLASH. (and asking for typos)

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

David,

Sorry, but I don't understand where are you aiming.

1. I am asking for comments on a proposed enhancement to strlen_P(). If a compiler would not recognize miltiplication by a constant which is power of 2, and I would propose a fix which would convert such multiplications to shifts, would you recommend just to teach users to use explicit shifts?

2. using sizeof() on strings as a methodology is asking for trouble - one day you'll forget (as you indeed did above) that they don't work on anonymous strings. The correct methodology for strings is to use the dedicated functions prototyped in string.h (and, here, for FLASH strings, pgmspace.h).

Do I misunderstand your comments?

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just to correct myself, my_strlen_P(PSTR("anything")) still results in function call, but the result is correct. A mere macro just could not fully replace a fully blown support for memory classes built into the compiler.

The following:

char s[] PROGMEM = "anything";
size = my_strlen_P(s);

results in size being loaded by constant; which is not the case if strlen_P(s) is used.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Looks fine. I use a similar method in LUFA for determining the bit pattern required for the endpoint configuration registers from the user-supplied bank size. If the input value is constant (via the same macro you use) the result is computed at compile time, otherwise it is replaced by a function call.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jan,

What do you actually want to achieve?

As I see it, I could equally well:

#define my_strlen(char_array) (sizeof(char_array)-1)

That macro would give you the string length of any flexible char array (initialised with a string). Both in Flash or SRAM.

It would always be a constant known at compile time. However it would not always be true if you modified a SRAM array. And it only works on arrays, not on pointers.

But I would assume that you would be using a regular strlen() on SRAM anyway. If you wanted the strlen_P() of a pointer that is known at compile time, you would use the sizeof(*ptr)-1. And it would be a lot clearer to reference the non-anonymous array in the first place.

If you wanted to know the length of a string that was pointed to by a SRAM pointer, you would have to use a function. It is not known at compile time.

I am guessing that you have a load of strings in Flash, and you want to have a table of the individual lengths.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Jan,

What do you actually want to achieve?


Better strlen_P().
david.prentice wrote:

As I see it, I could equally well:

#define my_strlen(char_array) (sizeof(char_array)-1)

That macro would give you the string length of any flexible char array (initialised with a string). Both in Flash or SRAM.


But, as shown above, that would fail with a string literal.

strlen()/strlen_P() both work both on string variables and literals.

As long as there are no generic pointers and true support for memory classes ("named sections" in gcc lingo) as we know it from the '51 and other small-mcu compilers, there's no real need to have functions working *both* in FLASH and RAM. The user HAS to be aware of the difference. The very moment when there will be such support, pgmspace.h will be past anyway.

David wrote:
But I would assume that you would be using a regular strlen() on SRAM anyway.

Exactly.

The user of avr_libc is guided to replace constant strings in RAM with constant strings in FLASH. At the same time, he is told to use the "_P" versions of functions traditionally prototyped in string.h; so, having been accustomised to use strlen() for strings in RAM, he should be seeking to use strlen_P() for strings in FLASH.

David wrote:
If you wanted the strlen_P() of a pointer that is known at compile time, you would use the sizeof(*ptr)-1.

Oh, no. sizeof(*PSTR("anything")) == sizeof(char) == 1.

David wrote:
And it would be a lot clearer to reference the non-anonymous array in the first place.
Sure; just you cannot exert your teacher's will on the users.

If you wanted to know the length of a string that was pointed to by a SRAM pointer, you would have to use a function. It is not known at compile time.

David wrote:
I am guessing that you have a load of strings in Flash, and you want to have a table of the individual lengths.

No. I have no use for it at the moment. I just came through this and though I will throw in a minor enhancement. I was naively under the impression that enhancements to avr-libc may be welcome.

Jan

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
Looks fine.
Thanks.

However, I still wait that somebody says "it will fail when XYZ"... ;-)

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm okay with the change, although I would like some kind of regression test. I believe that is what you are asking for, Jan.

I was originally going to say that I never use strlen_P, but then I did a search on my code. By golly, I use it! I'd forgotten about them completely!

There are two situations that I use it. First, is in my general output routines where I can pass a pointer to a PSTR back to the routine for printing:

   case cmdPT_PSTR:
      if ( ! g_MainAbortPending && ( *pBufLen >= strlen_P( (char*) pCmdPkt->param[parm] ) + 4 ) )
      {
         strcat_P( *ppBuf, (char*) pCmdPkt->param[parm] );
      }
      break;

The other place is where I have matched PSTR in an incoming string and want to skip over it:

   /* See if we got an error */
   pC = strstr_P( pSpbRxBuffer, SpbError );

   if ( pC != NULL )
   {
      /* Skip over the error flag and copy the string to the error string buffer */
      pC += strlen_P( SpbError );
      strcpy( pSpbErBuffer, pC );

      return SPB_TE_STRING_ERR;
   }

In the second case, your improvement would help.

Just a couple of RealLife(tm) examples to fuel the fires.

Stu

Engineering seems to boil down to: Cheap. Fast. Good. Choose two. Sometimes choose only one.

Newbie? Be sure to read the thread Newbie? Start here!

Pages