Skip to main content
gamedev.net
🔒 Locked

Pointers and Char

Started by siliconsurfer 13 replies 2.2k views
Original Post
siliconsurfer
siliconsurfer
Hi, I have problems with pointers and char's and can't just get my head around what's wrong! I have a function in C++ which reads in data from a file, I then want to compare the data read in, with a char* which is passed into the function. //The function loooks like this getCat(char *filename, char* inName); I declare a variable char *name; Then I read in the data using fgets(buffer, 255, fp); and read the data into the variable by doing sscanf(buffer, "%s", &name); Now I can print name to prove that the data is in there using printf("name = %s\n\n", &name); now I want to compare name with inName so I try if(strcmp(inName, name) == 0){ //do stuff } The problem is it crashes with a segmentation error. Any ideas what I'm doing wrong??
dimebolt
dimebolt
Quote:
Original post by siliconsurfer
printf("name = %s\n\n", &name);


One problem is that ampersand before name in the printf. It should be:
printf("name = %s\n\n", name);

otherwise it's trying to print the pointer value itself as a string...

There may be other problems in your function, but that is impossible to tell without the full code...

Tom
Enigma
Enigma
Well your main problem is that you've misspelt "string".:
void someNameBetterThanGetCatUnlessYouAreActuallyReturningAFeline(std::string filename, std::string expectedName){	std::ifstream reader(filename.c_str());	std::string name;	std::getline(reader, name);	if (name == expectedName)	{		// do stuff	}}

Enigma
siliconsurfer
siliconsurfer
Here is the complete code. The problem is when I try to do the strcmp on line 34. If I create a variable such as

char *newvar = "anystring";

And compare it with inUrl the code works fine. But comparing inUrl with domain causes the program to crash with a segmentation error. I presume this is to do with how sscanf works, but I can't figure out why, or how to fix it.


int getCat(char *filename, const char *inUrl)     18 {     19     FILE *fp;     20     char buffer[255];     21     22     char *domain;     23     //char *cat;     24     25     if((fp=fopen(filename, "r")) == NULL)     26     {     27         printf("Error opening file %s\n", filename);     28         return -1;     29     }     30     while(!feof(fp))     31     {     32         fgets(buffer, 255, fp);     33         sscanf(buffer, "%s", &domain);     34         if(strcmp(domain, inUrl) == 0){     35             printf("match found\n");     36             break;     37         }     38     39     }     40     fclose(fp);     41     42     return 0;     43 }

stylin
stylin
you declare domain as a pointer to char. You need an array:

char domain[ MAX_SIZE ];sscanf(buffer, "%s", domain);


Also, if you're doing this in C++, it's preferred to use std::string.
:stylin: "Make games, not war." "...if you're doing this to learn then just study a modern C++ compiler's implementation." -snk_kid
Nitage
Nitage
You don't allocate memory for domain.

If you're using C++ why are you using char* ?
Glak
Glak
if you have ANY trouble AT ALL using char* as a string type you should not be using it. The only time that you should use char* is when you are already quite experienced with std::string and have found one of those rare cases where using char* is better than using the real string type.

#include <string> //note, not string.h#include <iostream>#include <fstream>using namespace std;void GetFeline(string filename, string expected_data){    fstream fin(filename.c_str());    string actual_data;    fin>>actual_data;    if(actual_data==expected_data)    {        //blah blah blah    }}
siliconsurfer
siliconsurfer
I'm using char * because I'm working on code, which already uses char * rather than the string data structure.
It's also because I have to return a char * to a calling rpc.
rip-off
rip-off
need a std::string from a char*

no problem:

std::string str = char_ptr;



need a char* from a string

no problem:

do_something_with_a_char_pointer( str.c_str() );
blog | twitter
the_dannobot
the_dannobot
Here's how I would do it.
#define STRING_LENGTH 255int getCat(const char *filename, const char *inUrl)     18 {     19     FILE *fp;     20     char buffer[STRING_LENGTH];             if (filename == 0 || inUrl == 0)            {                return -1;            }            memset(buffer, 0, (sizeof(char) * STRING_LENGTH));     23     //char *cat;     24     25     if((fp=fopen(filename, "r")) == NULL)     26     {     27         printf("Error opening file %s\n", filename);     28         return -1;     29     }     30     while(!feof(fp))     31     {     32         fgets(buffer, STRING_LENGTH, fp);     34         if(strcmp(buffer, inUrl) == 0)                {     35             printf("match found\n");     36             break;     37         }     39     }     40     fclose(fp);     42     return 0;     43 }


[Edited by - the_dannobot on September 7, 2005 10:34:27 AM]
Enigma
Enigma
Quote:
Original post by the_dannobot
Here's how I would do it.
#define STRING_LENGTH 255

Ewww. Yuck. It was better before you edited and fell back on the gutter of the preprocessor. This is C++. At the very least use a const integral type rather than tromping on everybodies names. Also both domain and the memset are totally redundant. And why a STRING_LENGTH of 255 rather than 256?

Seriously, it's much more readable, maintainable and extensible to use the SC++L and convert from and to char * string only when accesing C api calls:
// assumes getSiameseOrEgyptianMau is to be passed as a callback function// to a C api, so uses char const * const parametersint getSiameseOrEgyptianMau(char const * const filenameCharArray, char const * const inUrlCharArray){	std::string filename(filenameCharArray);	std::ifstream reader(filename.c_str());	if (!reader)	{		std::cout << "Error opening file " << filename << '\n';		return -1;	}	std::string inUrl(inUrlCharArray);	std::string domain;	while (reader >> domain)	{		if(domain == inUrl)		{			std::cout << "match found\n";			break;		}	}	return 0;}

Enigma
the_dannobot
the_dannobot
Quote:
Original post by Enigma
Quote:
Original post by the_dannobot
Here's how I would do it.
#define STRING_LENGTH 255

And why a STRING_LENGTH of 255 rather than 256?

I have no idea. That's what the OP was using.
Enigma
Enigma
Ah, OK. Fair enough. But the gutter comment still stands [grin].

Enigma
Zahlman
Zahlman
Quote:
Original post by Enigma
// assumes getSiameseOrEgyptianMau is to be passed as a callback function// to a C api, so uses char const * const parametersint getSiameseOrEgyptianMau(char const * const filenameCharArray, char const * const inUrlCharArray){	std::string filename(filenameCharArray);	std::ifstream reader(filename.c_str());	if (!reader)	{		std::cout << "Error opening file " << filename << '\n';		return -1;	}	std::string inUrl(inUrlCharArray);	std::string domain;	while (reader >> domain)	{		if(domain == inUrl)		{			std::cout << "match found\n";			break;		}	}	return 0;}



Quoted for emphasis. This way of doing things:

- will work!
- easily interfaces with code that does use char*'s (shame on it)
- handles the memory management for you, in a way that won't limit your strings to any particular length (but also won't allocate a huge buffer if it isn't needed; there is a provable upper bound to the inefficiency)
- doesn't rely on hugely non-typesafe variadic functions (i.e. sscanf)
- is a bit shorter and definitely clearer
- is *definitely* more C++-idiomatic

The only thing I would change would be to construct the ifstream directly from the passed-in filename pointer; converting back and forth gains nothing here because we don't actually do any manipulation with that data. Oh, and inUrl doesn't need to be wrapped either since we just use it for comparison, and operator== is overloaded for (std::string, char*) appropriately. The real value here comes from using the local string variable, and the ability to insert into it directly from a new-style stream (FILE * is *ancient*! Why should we be thinking about this thing being a pointer rather than an object, anyway?)
Enigma
Enigma
Zahlman makes a good point about the needless conversions of the parameters, however there is one more issue that we both missed, so allow me to present a (hopefully) final revision:
// assumes getAmericanBobtailOrPersian is to be passed as a callback function// to a C api, so uses char const * const parametersint getAmericanBobtailOrPersian(char const * const filenameCharArray, char const * const inUrlCharArray){	if (!filenameCharArray)	{		std::cout << "filename is null\n";		return -1;	}	if (!inUrlCharArrray)	{		std::cout << "url is null\n";		return -1;	}	std::ifstream reader(filenameCharArray);	if (!reader)	{		std::cout << "Error opening file " << filename << '\n';		return -1;	}	std::string domain;	while (reader >> domain)	{		if(domain == inUrlCharArray)		{			std::cout << "match found\n";			break;		}	}	return 0;}

Enigma

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.