2

I need to convert arguments given at command line such as: $ myprogram hello world

and the words need to be printed in CAPS. I am able to to do everything except access the double pointer array to make the changes with toupper()

static char **duplicateArgs(int argc, char **argv)
{
    char **copy = malloc(argc * sizeof (*argv));
    if(copy == NULL){
        perror("malloc returned NULL");
        exit(1);
    }

    int i;
    for(i = 0; i<argc; i++){
         copy[i] = argv[i];   
    }
    char **temp;
    temp = &copy[1];
    *temp = toupper(copy[1]); 

    return copy;
}
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
Slider
  • 167
  • 1
  • 1
  • 8
  • 1
    Please indent your code properly. – Sourav Ghosh Feb 18 '15 at 06:21
  • 1
    Looks like you are using vim, the shortcut for indenting the code is `GG=g` – user3282276 Feb 18 '15 at 06:40
  • `char **temp; temp = &copy[1]; *temp = toupper(copy[1]); ` - what do you currently think this does? – user253751 Feb 18 '15 at 07:08
  • Also, note that while you're copying `argv` itself (which is a pointer to an array of pointers to strings), you're not copying the individual strings. – user253751 Feb 18 '15 at 07:09
  • Do you need "convert argument given at command line" or "convert argumentS given at command line"? As I see your example with `hello word` and `argc` parameter of `duplicateArgs` I suppose you want to process all the arguments, but accepted answer made me doubt – VolAnd Feb 18 '15 at 07:49

5 Answers5

3
*temp = toupper(copy[1]);

toupper converts a single character, if you want to convert an entire string:

char *temp = copy[1]; /* You don't need a double pointer */
size_t len = strlen(temp);

for (size_t i = 0; i < len; i++) {
    temp[i] = toupper(temp[i]); 
}
David Ranieri
  • 39,972
  • 7
  • 52
  • 94
1

From the man page of toupper() the function prototype is

int toupper(int c);

In your code, the argument copy[1] is not an int value.

Instead what you want is to check each and every element, if they are in lower case, convert them to upper case. A pseudo-code will look like

for(i = 0; i<argc; i++){
         copy[i] = malloc(strlen(argv[i])+ 1);  //allocate memory 

for (j = 1; j < argc; j++)
  for (i = 0; i < strlen(argv[j]); i++)
  {
    if (islower(argv[j][i]))   //check if it is lower case
         copy[j-1][i] = toupper(argv[j][i]);
    else
         copy[j-1][i] = argv[j][i];          //do not convert
  }
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • correct, because I am trying to convert to upper the characters inside the string which pointer copy[1] points to. How can I do it then? – Slider Feb 18 '15 at 06:28
  • @Slider Please Check the updated answer. It checks for lowercase and then converts. – Sourav Ghosh Feb 18 '15 at 07:27
  • @dear downvoter, please let me know the reason behind the downvote. Kindly consider leaving a comment so that the post can be improved. Thanks. – Sourav Ghosh Feb 18 '15 at 07:29
1

I assume the argument that is passed into your function char **argv is passed directly from main, so it represents a pointer to the beginning of an array of pointers to each of the command line arguments.

argc represents the number of command line arguments.

Inside your function, you create a new buffer, and then copy the contents of argv into it. So you are creating a copy of the array of pointers to the command line arguments, NOT the command line argument strings themselves.

I am guessing you intended to copy the strings, rather than the pointers to the strings (what would be the point of that?). I suggest you look into the functions strdup and/or strncpy to copy the actual strings.

This also explains with the 'toupper' does not work as you expect - instead of passing a single character to it, you are passing a pointer to a null terminated string of characters.

harmic
  • 28,606
  • 5
  • 67
  • 91
  • yes, I call the function with main, passing the argc and argv, in my function they come in as int argc and char **argv You are suggesting i copy the string, then apply toupper()?? – Slider Feb 18 '15 at 06:38
0

You are running into an error using the toupper() function because you are trying to pass in a string instead of an individual letter. Here is an excerpt from the man page describing the function:

DESCRIPTION
     The toupper() function converts a lower-case letter to the corresponding
     upper-case letter.  The argument must be representable as an unsigned
     char or the value of EOF.

You have a pointer to a pointer which you could visulize as something like this. In C a string is just an array of chars so you need to dereference twice to get the data in the second level of arrays (the individual letter). Every time you add an * you can think of it as removing one layer of pointers. And you can think of the * operator as the inverse of the & operator.

This line is your problem line

temp = &copy[1];

try this instead

//This is a pointer to an individual string
char *temp = copy[1];
//Keep going while there are letters in the string
while(*temp != NULL) {
    //convert the letter
    toupper(*temp);
    //Advance the pointer a letter
    temp++;
}
user3282276
  • 3,674
  • 8
  • 32
  • 48
  • `while (*temp != null)` should be `while (*temp != '\0')` – David Ranieri Feb 18 '15 at 11:30
  • @AlterMann I maybe should have used `\0` for clarity but it should work just the same the way I wrote it. `NULL` is typedefed to be 0 and `\0` is the octal number 0, both hold the same value are are thus equivalent. See here : http://stackoverflow.com/questions/1296843/what-is-the-difference-between-null-0-and-0 – user3282276 Feb 18 '15 at 15:51
0

Consider this example:

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

static char **duplicateArgs(int argc, char **argv)
{
    char **copy = NULL;
    // allocate memry for pointers to new lines
    copy = (char **)malloc(sizeof(char *) * argc);
    int line, chr;
    for(line = 0; line < argc; line++)
    {
        // allocate memory for new line
        copy[line] = (char *)malloc(sizeof(char) * (strlen(argv[line]) + 1));
        // copy with changes
        for(chr = 0; chr <= strlen(argv[line]); chr++)
        {
            copy[line][chr] = toupper(argv[line][chr]);
        }
    }
    return copy;
}

int main(int argc, char * argv[])
{
    char ** strs;
    int i;
    strs = duplicateArgs(argc, argv);
    for(i = 0; i < argc; i++)
    {
        printf("%s\n", strs[i]);
    }
    return 0;
}

EDIT:

Also you can make a decision about using argv[0] (name of executable file) and change a code if you need. Also checking of malloc result can be added, and other improvements... if you need :-)

VolAnd
  • 6,367
  • 3
  • 25
  • 43