-1

When I print out the length of the temp string, it starts at a random number. The goal of this for loop is to filter out everything that's not a letter, and it works for the most part, but when I print out the filtered string it returns the filtered string but with some extra random characters before and after the string.

#define yes 1000
...
char stringed[yes] = "teststring";
int len = strlen(text);

char filt[yes];

  for (int i = 0; i < len; i++) {
    if (isalpha(stringed[i])) {
      filt[strlen(filt)] = tolower(stringed[i]);
    }
  }

Jack A
  • 1,265
  • 2
  • 7
  • 16
  • 1
    Possible duplicate of [(Why) is using an uninitialized variable undefined behavior?](https://stackoverflow.com/questions/11962457/why-is-using-an-uninitialized-variable-undefined-behavior) – Joshua Sep 10 '19 at 16:16
  • 1
    btw. `temp[strlen(temp)] = "\0";` shouldn't compile and if you meant `temp[strlen(temp)] = '\0';` -- that wouldn't make sense either because what `strlen()` does is searching for `'\0'` – Ingo Leonhardt Sep 10 '19 at 16:18
  • 1
    It's not empty; it has exactly `MAX` elements, all with an indeterminate value. – molbdnilo Sep 10 '19 at 16:20
  • That edit is invalid C. You cannot declare a global variable with a non-constant initialiser. (`int length = strlen(text);`) – rici Sep 10 '19 at 16:29
  • @rici I'm not yet convinced it is global. The question still does not have an actual function defined so we can see how everything is scoped. – Christian Gibbons Sep 10 '19 at 16:37
  • @christian: yes, perhaps you're right. – rici Sep 10 '19 at 17:22
  • The first edit, Jack, was fine; it was made before the first answers were posted. The second edit is not OK; it eliminates code that was a key part of my answer, for example. Invalidating answers is not OK. If you need to change the code after an answer is posted, you need to add to the question, rather than replace the existing code. (I'm not sure why you think `yes` is better than `MAX` for the size of the array; I think `MAX` is better on several counts — meaningfulness of the name, and the use of upper-case rather than lower-case.) . – Jonathan Leffler Sep 10 '19 at 19:46

2 Answers2

3

There are at least two problems with the line:

temp[strlen(temp)] = "\0";
  1. The compiler should be shrieking about converting a pointer to an integer. You need '\0' and not "\0". (This might account for some of the odd characters; the least-significant byte of the address is probably stored over the null byte, making it and random other characters visible until the string printing comes across another null byte somewhere.)

  2. With that fixed, the code carefully writes a null byte over the null byte that marks the end of the string.

You should probably not be using strlen() at this point (or at a number of other points where you use it in the loop).

You should be using i more in the loop. If your goal is to eliminate non-alpha characters, you probably need two indexes, one for 'next character to check' and one for 'next position to overwrite'. After the loop, you need to write over the 'next position to overwrite' with the null byte.

int j = 0;   // Next position to overwrite
for (int i = 0; i < length; i++)
{
    if (isalpha(text[i]))
        temp[j++] = text[i];
}
temp[j] = '\0';
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
1

For starters the character array

char temp[MAX];

is not initialized. It has indeterminate values.

So these statements

printf("NUM:[%i] CHAR:[%c] TEMP:[%c] TEMPSTRLEN:[%i]\n", i, text[i], temp[strlen(temp)], strlen(temp));

  temp[strlen(temp)] = tolower(text[i]);

have undefined behavior because you may not apply the standard function strlen to uninitialized character array.

This statement

  temp[strlen(temp)] = "\0";

is also invalid.

In the left side of the assignment statement there is used the string literal "\0" which is implicitly converted to pointer to its first character.

So these statements

  length = strlen(temp);

  printf("[%s]\n", temp);

do not make sense.

It seems what you mean is the following

#include <stdio.h>
#include <string.h>
#include <ctype.h>

#define MAX 1000

int main(void) 
{
    char text[MAX] = "teststring";
    size_t length = strlen(text);

    char temp[MAX] = { '\0' };
    // or
    //char temp[MAX] = "";


    for ( size_t i = 0; i < length; i++) 
    {
        if (isalpha( ( unsigned char )text[i] ) ) 
        {
            printf("NUM:[%zu] CHAR:[%c] TEMP:[%c] TEMPSTRLEN:[%zu]\n", i, text[i], temp[strlen(temp)], strlen(temp));      

            temp[strlen(temp)] = tolower(text[i]);
            temp[i+1] = '\0';
        }
    }

  length = strlen(temp);

  printf( "[%s]\n", temp );

  return 0;
}

The program output is

NUM:[0] CHAR:[t] TEMP:[] TEMPSTRLEN:[0]
NUM:[1] CHAR:[e] TEMP:[] TEMPSTRLEN:[1]
NUM:[2] CHAR:[s] TEMP:[] TEMPSTRLEN:[2]
NUM:[3] CHAR:[t] TEMP:[] TEMPSTRLEN:[3]
NUM:[4] CHAR:[s] TEMP:[] TEMPSTRLEN:[4]
NUM:[5] CHAR:[t] TEMP:[] TEMPSTRLEN:[5]
NUM:[6] CHAR:[r] TEMP:[] TEMPSTRLEN:[6]
NUM:[7] CHAR:[i] TEMP:[] TEMPSTRLEN:[7]
NUM:[8] CHAR:[n] TEMP:[] TEMPSTRLEN:[8]
NUM:[9] CHAR:[g] TEMP:[] TEMPSTRLEN:[9]
[teststring]

Edit: next time do not change your question so cardinally because this can confuse readers of the question.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335