1

I just want to find a special sub-string in another string and save it as another string. Here is the code:

char sub[13]={};
char *ptr = sub;
char src[100] = "SOME DATE HERE CBC: 2345,23, SOME OTHER DATA";
//                              |----------|
ptr = strstr(src,"CBC:");
strncpy(sub,ptr,sizeof(sub)-1);

Is this an efficient way or does a better method exist for this? Thanks.

Iman H
  • 466
  • 6
  • 18
  • 1
    Simple and effective, I would say. No need for `ptr` I think? – Matthieu Brucher Oct 10 '18 at 08:46
  • empty braces are invalid C initialization. Are you using a C++ compiler? – pmg Oct 10 '18 at 08:47
  • `strncpy()`, despite its name, was not designed to work with strings. In your case, it will not write a `'\0'` to `sub[12]` *(which may already be there)*. – pmg Oct 10 '18 at 08:49
  • 3
    That should be: `if (ptr = strstr(src,"CBC:")) strncpy(sub,ptr,sizeof(sub)-1);` because the substring could not have been found. – Paul Ogilvie Oct 10 '18 at 08:49
  • @pmg, the string was initialized and so will have a zero at its end. – Paul Ogilvie Oct 10 '18 at 08:50
  • 1
    @PaulOgilvie: illegally initialized :) – pmg Oct 10 '18 at 08:52
  • Possible duplicate of [Strings in C, how to get subString](https://stackoverflow.com/questions/2114377/strings-in-c-how-to-get-substring) – YesThatIsMyName Oct 10 '18 at 09:01
  • Functions from the C standard library are normally optimized, so using `strstr` is probably more efficient than trying to roll your own... – Serge Ballesta Oct 10 '18 at 09:02
  • @MatthieuBrucher without ptr, we can not assign the strstr output. – Iman H Oct 10 '18 at 09:30
  • With `sub,src` lengths of B and C, a simple solution is `O(B*C)`. An effective solution is `O(B + C)`. Choose one. `strstr()` might be encoded either way. Recommend to just trust the lib. – chux - Reinstate Monica Oct 10 '18 at 14:35
  • "I just want to find a **special** sub-string in another string and **save** it as another string" Is it only me who thinks the task has no sense? Why not just set a flag? – Matt Oct 10 '18 at 15:01
  • @Matt `sub[]`and its _special data_ after the matching sub-string, may be needed long after `str[]` is changed. – chux - Reinstate Monica Oct 10 '18 at 15:05
  • @chux But that data is just a constant string. In practice it's almost always easier to operate with flags/indexes into arrays of literals. – Matt Oct 10 '18 at 15:12
  • @Matt As a typical [mcve], it is reasonable to assume `char src[]` is not constant but only given the value of `"SOME DATE HERE CBC: 2345,23, SOME OTHER DATA"` to illustrate the issue. – chux - Reinstate Monica Oct 10 '18 at 15:15
  • @Matt The data after CBC: is changing every 1 seconds. It should be extracted and then interpreted in some way. – Iman H Oct 11 '18 at 05:09
  • 1
    @ImanH Still I don't see the need to save the literal prefix "CBC: ". Also the intepretation of a 13-byte string should not be too elaborate, so it could probably be done in-place effectively eliminating any need to save it. – Matt Oct 11 '18 at 07:50

2 Answers2

1

Is this an efficient way or does a better method exist for this?

Pros:

Uses strstr() which is likely more efficient and correct that coding your own search.

Cons:

Does not handle the case when strstr() return NULL resulting in undefined behavior (UB). @Paul Ogilvie

ptr = strstr(src,"CBC:");
// add test
if (ptr) {
  // copy
} else {
  // Handle not found, perhaps `sub[0] = '\0';`
}

char sub[13]={}; is not compliant C code. @pmg. A full initialization of the array is not needed - even though it is a common good practice.

Code does not quite fulfill "want to find a special sub-string in another string and save it as another string". Its more like "want to find a special sub-string in another string and save it and more as another string".

strncpy(sub,ptr,sizeof(sub)-1) can unnecessarily mostly fill the array with null characters. This is inefficient when ptr points to a string much less than sizeof(sub). Code could use strncat() but that is tricky. See this good answer @AnT.

// alternative
char src[100] = "SOME DATE HERE CBC: 2345,23, SOME OTHER DATA";

char sub[13];
sub[0] = '\0';

const char *ptr = strstr(src, "CBC:");
if (ptr) {
  strncat(sub, p, sizeof sub - 1);
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • _strncpy(sub,ptr,sizeof(sub)-1) can unnecessarily mostly fill the array with null characters._ The memory for _src_ is allocated so far. Is the inefficiency in its computational complexity? – Iman H Oct 11 '18 at 05:23
1

Unless this piece of code is in a critical path and it's the actual source of a performance bottleneck, then just stick to what you have. Default strstr implementation should be quite adequate for the task.

You can squeeze some peanuts by pre-computing the end of src, for example, so you could use memcmp (an unconditional loop) instead of strncpy (which has a conditional loop) when extracting the sub. If you know beforehand the substring you are searching for, you can optimize around that too; especially if it's exactly 4 characters. And so on and so forth.

But if you are after these peanuts, you might be (much) better off by redoing the code to not extract sub to begin with and use something like ranged strings to keep track of where it is in the source string.

Angstrom
  • 352
  • 1
  • 2
  • 16