Output wrong. Possible strncpy issue?

Question

So, I'm trying to get this code to parse each line inputted from the file into individual tokens, then add each one in turn to tklist array. Then the main just prints out each token. It's printing blanks though, and when I step into the code it looks like the strncpy isn't working. Any ideas what the issue is? I get no errors.

Here's the main function:

#include <iostream>
#include <fstream>
using namespace std;

#include "definitions.h"
#include "system_utilities.h"


int main()
{
    ifstream inFile;

    char line[MAX_CMD_LINE_LENGTH];
    char* token[MAX_TOKENS_ON_A_LINE];
    int numtokens;

    system("pwd");
    inFile.open("p4input.txt", ios::in);
    if(inFile.fail()) {
        cout << "Could not open input file.  Program terminating.\n\n";
        return 0;
    }
    while (!inFile.eof())
    {
    inFile.getline(line, 255);
    line[strlen(line)+1] = '\0';
    numtokens = parseCommandLine(line, token);
        int t;
        for (t=1; t <= numtokens; t++) {
            cout << "Token "<< t << ": " << token[t-1] << "\n";
        }

    }
    return 0;
}

And here's the parseCommandLine function:

int parseCommandLine(char cline[], char *tklist[]){
    int i;
    int length; //length of line
    int count = 0; //counts number of tokens
    int toklength = 0; //counts the length of each token
    length = strlen(cline);
    for (i=0; i < length; i++) {   //go to first character of each token

        if (((cline[i] != ' ' && cline[i-1]==' ') || i == 0)&& cline[i]!= '"') {

        while ((cline[i]!=' ')&& (cline[i] != '\0') && (cline[i] != '\r')){
            toklength++;
            i++;
        }
      //---------------
    tklist[count] = (char *) malloc( toklength +1);
    strncpy(tklist[count], &cline[i-toklength], toklength);
    //--------------
        count ++;
        toklength = 0;
    }

    if (cline[i] == '"') {
        do {
            toklength++;
            i++;
            if (cline[i] == ' ') {
                toklength--;
            }
        } while (cline[i]!='"');

        //--------------
        tklist[count] = (char *) malloc( toklength +1);
        strncpy(tklist[count], &cline[i-toklength], toklength);
        //--------------
        count ++;
        toklength = 0;
    }

}
int j;
for (j = 0; j < count; j++) {
    free( (void *)tklist[j] );
}
return count;

}

Like I said, when I debug it looks like a problem with copying, but I'm a beginner so I suspect I'm doing something wrong.

Thanks for any help you can give!!

`strncpy(tklist[count], &cline[i-toklength], toklength);` doesn't 0-terminate, you need to do that. `line[strlen(line)+1] = '\0';` is pointless, there's already a 0 byte at `line[strlen(line)]`. — Daniel Fischer, May 13 '13 at 21:43
Your main problem is that you are supposed to be writing C++ code but you're mostly using C - malloc/free, strncpy, etc. Your code would be simpler, cleaner and more robust if you took the time to learn C++ properly. — Paul R, May 13 '13 at 21:44
This is part of an assignment, as I am learning C++, but have some basic experience in C. I have to use malloc/free, but I could use memcpy (I think that's c++) instead of strncpy if you think that would make a difference. . . — singmotor, May 13 '13 at 21:58

codah · Accepted Answer · 2013-05-13T22:14:01.563

1

Try something like

tklist[count][toklength]='\0';

after

strncpy(tklist[count], &cline[i-toklength], toklength);

strncpy() does not necessarily add a null terminator for you. strncpy needs some care to use safely.

No null-character is implicitly appended at the end of destination if source is longer than num..

Just for starters... there are other deeper issues as mentioned in comments.

edited May 13 '13 at 22:14

answered May 13 '13 at 21:55

codah

466
4
14

Thanks @codah! Like you and @Daniel Fischer said, adding that null character helped with some of the problems. However, now it successfully calls the function once (for one line of the input) and then has issues with the second line. It cuts off the first char of the second token, and after printing the final token says:Program 4(3258) malloc: *** error for object 0x100103ae0: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug – singmotor May 13 '13 at 22:15
Yes I indicated it was just for starters.. Unfortunately I'm at work and can't post a comprehensive solution. Best way to use strncpy safely is to create a larger buffer than you need for the strncpy, and use (size-of-buffer) -1 in your 2nd argument to strncpy. – codah May 13 '13 at 22:26
Problem solved! Thanks for your help though @codah. My error issue turned out to be with my free statement. I got rid of the freeing at the end of the function and just added free((void *)token[t-1]); after the function call in main and now everything is working! Thank you everyone for all of your help though. As a beginner I really appreciate it, and will be sure to give back once I have knowledge under my belt. – singmotor May 14 '13 at 02:42

OOhay · Answer 2 · 2013-05-13T22:33:39.023

0

To start with the generic equivalent of malloc/free is new/delete (heap memory allocation).

Second you seem to be confusing strings and c_strings (good old char*). getline uses strings, your parsing function uses c_strings they are not the same things and there is .c_str() a member function of string to do the conversion.

edited May 13 '13 at 22:33

answered May 13 '13 at 22:28

OOhay

61
6

score 0 · Answer 3 · answered May 13 '13 at 22:50

0

So, I'm trying to get this code to parse each line inputted from the file into individual tokens, then add each one in turn to tklist array.

For

each line inputted from the file

use

std::ifstream ifs;
std::string s;
/**/
std::getline(ifs, s);

adopted to your loop.

To

parse [..] into individual tokens

look how std::string can help you on that task (or use boost::tokenizer).

And this

then add each [token] in turn to tklist array.

almost cries for std::list or std::vector instead of a plain C array, the choice, which container to use depends e.g. on what you intend to do with the tokens found.

answered May 13 '13 at 22:50

Solkar

1,228
12
22

Haha yea definitely, the catch to the assignment was that I can't use the string class :/ that's why I'm doing it with the array – singmotor May 14 '13 at 02:39
It's by no means funny - you tagged this as c++ and your code a kind of a role model of how NOT to do it with that language. – Solkar May 14 '13 at 09:35

Output wrong. Possible strncpy issue?

3 Answers3