-2

I'm new to C and I don't seem to know how to set my pointer *year to NULL before calling for atoi. I have this code below that separates my string into 4 and when I run it it throws off the program and I'm sure it's because of the atoi function i inserted. Could you please help me?

 char *split(char words[50])
 {
int i = 0;
char* words_dup = malloc(strlen(words)+1);
strcpy(words_dup,words);

while (words!='\0')
{
    char *word=strtok(words, "_#_");
    check_word(*word);

    char *year=strtok(NULL, "_#_");;  // assigning NULL for previousely where it left off
    i=atoi(year);
    check_year(i);

    char *definition=strtok(NULL,"_#_");
    check_definition(*definition);

    char *synonyms=strtok(NULL,"_#_");
    check_synonyms(*synonyms);


    printf("%s\t", word);
    printf("%i\t",i);
    printf("%s\t", definition);
    printf("%s\t", synonyms);
}

// now restore words
strcpy(words,words_dup);
free(words_dup);
return 0;
 }

 //------------------------------------------------------------------//
 //                     CHECKING LEGAL OR NOT
 //------------------------------------------------------------------//
 void check_word(char word)
 {
if (word>='A' && word<='Z')
{
    printf("not legal\n");
}
 }


 void check_year(int year)
 {
if (year<0)
{
    printf("not legal\n");
}
 }

 void check_definition(char definition)
 {
if (definition>='A' && definition<='Z')
{
    printf("not legal\n");
}
 }

 void check_synonyms(char synonym)
 {
if (synonym>='a' && synonym<='z')
{
    printf("not legal\n");
}
 }


 int main()
 {
char words[100];
printf("Enter a string\n");
scanf("%s", words);
split(words);
 }

This is what I'm trying to input: hello_#_2003_#_now_#_MY

Alkis Kalogeris
  • 17,044
  • 15
  • 59
  • 113
user2985083
  • 63
  • 1
  • 8
  • 2
    Why would you want to set the pointer to `NULL` before calling `atoi()` on it? That's an instant segfault (or whatever undefined behavior manifests in)... –  Jan 05 '14 at 11:39
  • 2
    strtok does not break a string on a string like `"_#_"`, it breaks on a list of single characters, see this web site for examples: https://www.google.de/search?output=search&q=strtok+example – Brandin Jan 05 '14 at 11:41
  • @H2CO3 well when I run the code it tells me 'EXE_BAD_ACCESS(code=1,address=(0x0))' – user2985083 Jan 05 '14 at 11:44
  • @user2985083 And is that what you want? –  Jan 05 '14 at 11:48
  • I just want it to have 2003 as an int – user2985083 Jan 05 '14 at 11:50
  • 1
    `while (words!='\0')` Infinite loop、 – BLUEPIXY Jan 05 '14 at 11:59
  • done it but it still get's stuck on my atoi – user2985083 Jan 05 '14 at 12:04
  • @Brandin true about strtok, but it should work as expected for the user since «A sequence of two or more contiguous delimiter bytes in the parsed string is considered to be a single delimiter.» (man strtok). – ShinTakezou Jan 05 '14 at 14:18

3 Answers3

1

You shouldn't use atoi since it does not allow you to know if it wasn't able to parse its input, but returns an int anyway. But for the sake of the exercise, let us suppose it's ok since the input is good (i.e. a signed integer).

Then, some notes about your code:

split does not need to return char *, in fact you call it as it would return void, so change split into void split…; in any case, keeping char *split…, it should return NULL, not 0.

These lines

char* words_dup = malloc(strlen(words)+1);
strcpy(words_dup,words);

can be simply

char *words_dup = strdup(words);

Then,

while (words!='\0')

words is a pointer (char *), not a char, so it should be *words or maybe you meant words != NULL, but words isn't changed into the loop, so you will loop forever.

I suppose then you mean to parse a "stream" of "chunks" containing word, year, definition, synonym, in a continuous stream.

If it is so, you have to word = strtok(words, "_#_") only once, and then use word = strtok(NULL, "_#_"). You could achieve this with something like

 char *word;

 // use words_dup to avoid changing words.
 for (word = strtok(words_dup, sep); word != NULL; word = strtok(NULL, sep))
 {
        check_word(word);
        // ...
 }

(You should in general check if strtok returned NULL each time you try to get a new piece of information; you can exit from the for loop using break, if you need).

If it is not so, you don't need the loop at all: call "split" for each line of your input, if you have several lines.

The use of "_#_" works as you wanted not for the reason that likely you think (that argument is a set of character, each considered a separator alone), but since:

A sequence of two or more contiguous delimiter bytes in the parsed string is considered to be a single delimiter

The code

char *year=strtok(NULL, "_#_");;  // assigning NULL…
i=atoi(year);
check_year(i);

is fine, provided that

  1. we ignore atoi, since its usage is not recommended: input must be "good"
  2. we don't need to know the outcome of the check — which sounds strage (why we check?)

If you are worried about corrupting words, use words_dup in the first strtok, and at the end you need only to free it!

free(words_dup);

functions

check_word is a misleading name: you check just the first character (in the other functions too).

check_* functions do not return anything, so you can't take decision in the caller about the outcome of the check: the "word" is "not legal", then what it is supposed the caller (split) should do? Your program goes on, and the user read not legal, without s/he can realize exactly what is not legal: the "word" or the "year", or the "definition" (again, its first char)…?

Is the string all lower case?

(Do you wanted this? Then…)

You should write a function like this

int is_lowecase(const char *str)
{
    while (*str != '\0') {
       if (*str < 'a' || *str > 'z') return 0;
       str++;
    }
    return 1; // anything != 0 is "read" as true
}

which returns 1 iff all the chars are lowercase (this is not the better way of doing it, but I am supposing you can't use islower).

Then the caller, e.g. check_word, could be:

 int check_word(const char *word)
 {
   // only all lowercase words are legal
   if (!is_lowercase(word)) {
      // fprintf(stderr, "...");
      // indeed I think it should be a caller duty to print
      // such messages
      printf("word: not legal\n");
      return 0;
   }
   return 1;
 }

The caller can take into account the return code and decide to stop the parsing (break) on an illegal part. If you don't need it, change back to your void func(...) signature. The idea otherwise is:

 if (!check_A_Test(datum)) {
     // false (0) is Wrong, true (!= 0) is Right, thus the !
     // message here, if you want
     break; // stop parsing
 }

(If const is unknown to you, remove it).

You can use the very same function is_lowercase for check_definition and check_synonyms.

ShinTakezou
  • 9,432
  • 1
  • 29
  • 39
  • This info is more than useful. to add to it you asked questions regarding on how i should input my string basically the string has to be like this: (word)_#_(year)_#_(definition)_#_(SYNONYMS(UPPER-CASE)) – user2985083 Jan 05 '14 at 18:55
  • currently the answer is modelled according to the requirements that string inputs are valid only if each letter is lowercase. If synonyms must be uppercase, then you need another func `is_uppercase` to check synonyms: it is very similar to `is_lowercase`. If you have still trouble with your atoi, try adding some log, e.g. print `year` before the call to `atoi`, so you know what you are giving to `atoi` as argument. – ShinTakezou Jan 05 '14 at 19:27
0

You should not use atoi but strtol(3) with a non-null end pointer (or sscanf(3)...) :

long i;
char* end = NULL;
i = strtol(year, &end, 0);
if (i>0 && end>year) {
  check_year(i);
  // do something
} 
else {
  fprintf(stderr, "bad year: %s\n, year);
  exit(EXIT_FAILURE);
}

Then do something sensible with end, perhaps passing it to strtok or probably to strstr(3) or strchr(3) to find the relevant next character[s]...

And I believe your usage of strtok(3) is wrong. Maybe consider sscanf(3) perhaps with a %n. See this answer to a similar question.

I don't understand what exactly is your input in general, but perhaps you just want:

bool parse_line(const char*line, int* pyear)
{
   int pos= -1;
   int year=0;
   if (sscanf(line, "%*[a-z]_#_%d_#_%*[a-z] %n", &year, &pos)>0 && pos>0
              && pos == strlen(line)) {
      *pyear = year;
      return true;
   } else return false;
}

that you would later call with (after reading an entire line with getline(3) or else fgets(3))

int yearnum=0;
if (parse_line(line, &yearnum)) 
   check_year(yearnum);
else printf("not legal\n");

Don't forget to compile with all warnings and debug info (e.g. gcc -Wall -g) and to use the debugger (e.g. gdb), notably to run step by step your buggy program!

Community
  • 1
  • 1
Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
  • Thank you but i'm not allowed to use what you showed me. my input is hello_#_2003_#_now_#_MY and the condition is if the year is negative print: not legal – user2985083 Jan 05 '14 at 12:05
  • 1
    Then don't use `strtok` unless you have to and parse manually your line (but do use `strtol` with an `end` pointer). And **learn how to use your debugger** – Basile Starynkevitch Jan 05 '14 at 12:23
0

sample

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

char *str_dup(const char *str){//use strdup if exist strdup
    char *sdup = malloc(strlen(str)+1);
    if(sdup)
        return strcpy(sdup, str);
    return NULL;
}
char *strtokByWord_r(char *str, const char *word, char **store){
    char *p, *ret;
    if(str != NULL){
        *store = str;
    }
    if(*store == NULL) return NULL;
    p = strstr(ret=*store, word);
    if(p){
        *p='\0';
        *store = p + strlen(word);
    } else {
        *store = NULL;
    }
    return ret;
}
char *strtokByWord(char *str, const char *word){
    static char *store = NULL;
    return strtokByWord_r(str, word, &store);
}

typedef struct record {
    char *word;
    int year;
    char *definition;
    char *synonyms;
} Record;

Record *get_rec(const char *input_string){
    Record *rec;
    char* words_dup = str_dup(input_string);
    rec = malloc(sizeof(Record));

    char *word=strtokByWord(words_dup, "_#_");
    //if(check_word(word)!=Valid)
    //{ fprintf(stderr, "bad format\n"); free(rec);/*free(words_dup);...*/ return NULL;}
    rec->word = str_dup(word);

    char *year=strtokByWord(NULL, "_#_");
    rec->year =atoi(year);

    char *definition=strtokByWord(NULL,"_#_");
    rec->definition = str_dup(definition);

    char *synonyms=strtokByWord(NULL,"_#_");
    char *newline = strchr(synonyms, '\n');
    if(newline)
        *newline = '\0';
    rec->synonyms = str_dup(synonyms);

    free(words_dup);

    return rec;
}

int main(void){
    const char *input = "hello_#_2003_#_now_#_MY\n";
    Record *r = get_rec(input);
    printf("%s\t", r->word);
    printf("%i\t",r->year);
    printf("%s\t", r->definition);
    printf("%s\n", r->synonyms);
    free(r->word);free(r->definition);free(r->synonyms);
    free(r);
    return 0;
}

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

bool is_lowers(const char *str){
    for(; *str ; ++str)
        if(!islower(*str))
            return false;
    return true;//if call is_lowers("") then should be return value is true or false ?
}

int main(void){
    char word[] = "word";
    if(is_lowers(word))
        printf("valid\n");
    else
        printf("invalid\n");
    word[0] = 'W';
    if(is_lowers(word))
        printf("valid\n");
    else
        printf("invalid\n");
    return 0;
}
BLUEPIXY
  • 39,699
  • 7
  • 33
  • 70
  • thank you. I have one issue. I would like to put a condition for word and say that if word is in upper case then print not legal but I see here that I'm comparing a char and int which is not valid – user2985083 Jan 05 '14 at 12:43
  • @user2985083 I have added a sample. – BLUEPIXY Jan 05 '14 at 12:59