2

I am currently facing a problem while coding. The problem is I want to loop through a string and compare each index with another string's index. And at the same time, copy the character to the other string if it does not have it yet. The code below is where I got stuck:

I compiled this and got the error: comparison between pointer and integer ('char' and 'string' (aka 'char *')) [-Werror,-Wpointer-integer-compare]

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

int main(int argc, string argv[1])
{
    string key = argv[1], key2[26];

 for (int i = 0; key[i] != '\0' ; i++)
    {
        int j = 0;

        if (key[i] != key2[j]) // I got an error here
        {
            key2[j] = key[i];
            j++
        }
    }
    printf("%s\n", key2);
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
ordigam
  • 21
  • 3

2 Answers2

0

This shows why you shouldn't hide a pointer behind a typedef and why hiding how C works from novice programmers doesn't benefit anyone.

There is no string type in C. string here is just a typedef for a char *. The following statement:

string key = argv[1], key2[26];

allocates memory for a char *, and an array[26] of char *, which are not synonymous.

Or in other words: It declares:

char *key = argv[1];
char *key2[26];

Note that the these statements only allocated memory for the pointers, not what they point to. In this case, you did initialize key, but forgot to initialize key2.

The statement:

if (key[i] != key2[j])

is then invalid. Because key[i] evaluates to a char, where key2[j] evaluates to a char * (that you didn't allocate memory for, nor initialize).


Fix:

Simply declare key2 to be a char *, not an array of char *s.

Now find the length of argv[1] before copying (it could result in an overflow elsewise).

size_t len = strlen (argv[1]);

Now allocate memory for key2:

char *key2 = malloc (len + 1); /* One for the '\0' byte */
if (!key2) {
    /* ENOMEM, handle error accordingly */
} 

Now copy the strings with strcpy() and continue on with the loop. And free() the memory when you're done with it.


Minor: You are accessing argv[1] before checking if its valid.

 if (argc != 2) {
     fprintf (stderr, "/* Print usage message here */.\n");
     return EXIT_FAILURE;
 }                                                                                                 
Harith
  • 4,663
  • 1
  • 5
  • 20
0

An array of 26 char is needed, not 26 string

// string key = argv[1], key2[26];
string key = argv[1];
char key2[26];

key2[] not initialized

Unclear how OP wants to populate this. Perhaps from next argument?

Best to test argc first

if (argc < 2) {
  fprintf(stderr, "Missing argument\n");
  return EXIT_FAILURE;
}
string key = argv[1];
...

Drop unneeded 1

// int main(int argc, string argv[1])
int main(int argc, string argv[])
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256