views:

55

answers:

3

Here is my code:

/*
 * main.c
 *
 *  Created on: 15 Oct 2010
 *      Author: mohit
 */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/dir.h>
#include <sys/unistd.h>

void print_usage();

int main(int argc, char *argv[])
{
    int i;
    char *directory;
    char *name;

    //Iterate through command line arguments
    for (i = 1; i < argc; i++)
    {
    if (!strcmp(argv[i], "-d") || !strcmp(argv[i], "--directory"))
    {
        memcpy(directory, argv[i + 1], sizeof (argv[i + 1]));
        continue;
    }

    else if (!strcmp(argv[i], "-n") || !strcmp(argv[i], "--name"))
    {
        memcpy(name, argv[i + 1], sizeof (argv[i + 1]));
        continue;
    }
}

if (!name)
{
    perror("\n\nIncorrect Usage! Name was not provided!\n\n");
    print_usage();
}
if (directory)
    chdir(directory);

printf("\nCreating %s.theme directory...\n", name);
mkdir(strcat(name, ".theme"), "a+rw");
printf("Created %s.theme...\n", name);

printf("Entering %s.theme...\n", name);
chdir(strcat(name, ".theme"));

printf("Creating Icons directory...\n");
mkdir("Icons", "a+rw");
printf("Created Icons...\n");

printf("Creating Bundles directory...\n");
mkdir("Bundles", "a+rw");
printf("Created Bundles...\n");

printf("Creating UIImages directory...\n");
mkdir("UIImages", "a+rw");
printf("Created UIImages...\n");

printf("Creating Folder directory...\n");
mkdir("Folder", "a+rw");
printf("Created Folder...\n");

return 0;
}

void print_usage()
{
printf("\n\nUsage: wbt-create [--directory directory] --name theme_name\n");
printf("\n\n\t wbt-create [-d directory] -n theme_name\n");
}
+1  A: 
char *directory;
char *name;

Those are declaring pointers, but not allocating any memory for them. When you do the memcpy() to copy data to those pointers you will get the Seg fault.

You could declare char arrays on the stack instead, something like:

char directory[100];

or you will have to use malloc() to allocate the memory yourself.

ghills
Why allocate unnecessary memory? `argv` isn't going anywhere.
Michael Mior
@Michael For this exact case, sure that will work. But in general one should understand why that segfault was occurring and that is what I wanted to point out.
ghills
A: 

Change:

int main(int argc, char *argv[])
{
    int i;
    char *directory;
    char *name;

    //Iterate through command line arguments
    for (i = 1; i < argc; i++)
    {
        if (!strcmp(argv[i], "-d") || !strcmp(argv[i], "--directory"))
        {
            memcpy(directory, argv[i + 1], sizeof (argv[i + 1]));
            continue;
        }

        else if (!strcmp(argv[i], "-n") || !strcmp(argv[i], "--name"))
        {
            memcpy(name, argv[i + 1], sizeof (argv[i + 1]));
            continue;
        }
    }
    ...

to:

#include <limits.h>

int main(int argc, char *argv[])
{
    int i;
    char directory[PATH_MAX];
    char name[PATH_MAX];

    //Iterate through command line arguments
    for (i = 1; i < argc; i++)
    {
        if (!strcmp(argv[i], "-d") || !strcmp(argv[i], "--directory"))
        {
            strcpy(directory, argv[i + 1]);
            continue;
        }

        else if (!strcmp(argv[i], "-n") || !strcmp(argv[i], "--name"))
        {
            strcpy(name, argv[i + 1]);
            continue;
        }
    }
    ...
Paul R
+2  A: 

Arguments in argv will stay allocated while your program is running. Therefore, you can avoid the memcpy (and the allocation of memory) and simply assign the pointers. Example below:

...
int main(int argc, char *argv[])
{
    int i;
    char *directory;
    char *name;

    //Iterate through command line arguments
    for (i = 1; i < argc; i++)
    {
    if (!strcmp(argv[i], "-d") || !strcmp(argv[i], "--directory"))
    {
        directory = argv[i + 1];
        continue;
    }

    else if (!strcmp(argv[i], "-n") || !strcmp(argv[i], "--name"))
    {
        name = argv[i + 1];
        continue;
    }
}
...

However, I would highly recommend the use of getopt. It's a fairly standard library and handles these kind of things quite well.

Michael Mior
Thanks for the tip. getopt works really well.
Mohit Deshpande
No problem. Good luck! :)
Michael Mior