0

I am trying to figure out how do I use the data that I've entered in the command line and use it to convert my data into uppercase letters. I am very new in C. Here is the code. Can anyone tell me what I did wrong? I am trying to convert any lowercase words to uppercase by using dynamic memory allocation.

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


int main(int argc, char*argv[]){
int i;
char * string;
//Dynamic Memory Allocation

string = (char*)malloc(argc * sizeof(char));
for (i=1; i<argc; i++){
printf("%s\n",argv[i]);

}       

strcpy(string, argv[i]);

for (i = 1; string[i] != '\0'; i++){
    if(string[i] >= 'a' && string[i] <= 'z'){
        string[i] = string[i] - 32;
    }
}

return 0;
}
Devilicious
  • 1
  • 1
  • 1

2 Answers2

2

There are a few errors

  1. Firstly

    string = (char*)malloc(argc * sizeof(char));

argc gives the no of parameters, not the length of the string. To get the length of the argv[1] parameter, use strlen

len = strlen(argv[1]);
string = malloc (len+1);

  1. Secondly

    strcpy(string, argv[i]);

argv[0] is usually the program name. argv[1] will give the name of the first parameter. You can also add a check on argc to see if the user has added no of parameters. If you are entering one parameter, this can be changed to,

if (argc == 2)
{
   strcpy(string, argv[1]);
}
else
{
   //report error to user
}
  1. Thirdly

You are referring the indexes starting from 1. In C, the indexes start from 0.

for (i = 0; string[i] != '\0'; i++){
    if(string[i] >= 'a' && string[i] <= 'z'){
        string[i] = string[i] - 32;
    }
}
Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31
1

Can anyone tell me what I did wrong?

Yes. Various problems.

string = (char*)malloc(argc * sizeof(char)); allocates based on the number of arguments (each a string) to the program and not to the lengths of the strings involved. To find the length of a string use size_t len = strlen(s); Add 1 for the string size. @David C. Rankin

i in strcpy(string, argv[i]) is the same value as argc and then argv[argc] == NULL. strcpy(string, argv[i]) is bad, undefined behavior, as argv[i] does not point to a string.

string[i] >= 'a' && string[i] <= 'z') is a weak way to detect uppercase characters. Use isupper() for performance and portability.

string[i] = string[i] - 32; is a weak way to convert to upper case. Use toupper() for performance and portability and then isupper() is not needed.

string = (char*)malloc(argc * sizeof(char)); uses an unnecessary cast. Easier to code right, review and maintain code to size to the de-referenced pointer than to the pointer type. string = malloc(sizeof *string * n);

A classic approach would allocate an array of pointers to strings and then memory for each string. Then convert those to uppercase using standard library functions.

// Get memory for the string pointers
char **uppercase_string = malloc(sizeof *uppercase_string * argc);
if (uppercase_string == NULL) {
  fprintf(stderr, "Out of memory");  
  return EXIT_FAILURE;
}

// Get memory for each string and convert
for (int a=0; a< argv; a++) {
  size_t length = strlen(argv[a]);
  size_t size = length + 1;  // strings always have a final null character.
  uppercase_string[a] = malloc(sizeof *uppercase_string[a] * size);
  if (uppercase_string[a] == NULL) {
    fprintf(stderr, "Out of memory");  
    return EXIT_FAILURE;
  }
  for (size_t i = 0; i < length; i++) {
    uppercase_string[a][i] = toupper((unsigned char) argv[a][i]);
  }
  uppercase_string[a][length] = '\0'; // append null character.
}

// Use uppercase_string somehow
for (int a=0; a< argv; a++) {
  printf("%d <%s>\n", a, uppercase_string[a]);
}

// free resources when done
for (int a=0; a< argv; a++) {
  free(uppercase_string[a]));
}
free(uppercase_string);

[Advanced]

C allows the strings pointed to by argv[i] to be modified.

"The parameters argc and argv and the strings pointed to by the argv array shall be modifiable by the program, and retain their last-stored values between program startup and program termination." C11dr §5.1.2.2.1 2

Although I consider it weak practice to mod argv[], code could forego the memory allocation.

for (int a= 1; a < argc; a++) {  // start at 0 it you want to convert the program name too.
  char *string = argv[a];
  for (size_t i = 0; string[i]; i++) {
    if(string[i] >= 'a' && string[i] <= 'z') {
        string[i] = string[i] - 32;
    }
  }
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • "strings pointed to by the argv array shall be modifiable"... Concur with modification being a weak practice. There being no guarantee, I've wondered whether and if there is any standard or agreed upon storage size for the strings. I've yet to stumble across an answer. – David C. Rankin Nov 06 '18 at 07:29
  • @DavidC.Rankin There is the spec (guarantee) that says the string can be modified. Yet reaching beyond the original null character is UB. Interestingly - it appears modifying `argv[a]` is UB, although changing `argv` and `argv[a][i]` OK. [ref](https://stackoverflow.com/a/25747537/2410359) – chux - Reinstate Monica Nov 06 '18 at 07:35
  • best advise and solid answer "Although the strings are modifiable, code should not exceed the original string's length." That, at least, does not run afoul of "any explicit definition of behavior". – David C. Rankin Nov 06 '18 at 07:39