I have the following c++ code
char **filename is passed and is allocated in the calling function as
char *filename = malloc(1000);
int randominteger;
char *zptr;
char qfilename[500];
char randomintegerchar[500];
char *mimetype;
char *tptr;
char *cfilepath;
mimetype = part->getMimeType(); //Get the file type
tptr = strrchr(mimetype, '/'); //Strip off the extra values from the
getmimetype call
strcpy(qfilename, "BaseTempFile"); //Start building the temp
filename
randominteger = rand(); ///Get a random number
sprintf(randomintegerchar, "%d.", randominteger); //Turn the number
into a string
//Build the filename
strcat(qfilename, randomintegerchar); //Add the random number to make
the filename to make it unique
strcat(qfilename, tptr+1);
zptr = qfilename;
strcpy(*filename, zptr); //passed back to update the import name
strcat(cfilepath,"\\");
strcat(cfilepath, *filename);
part->getContentToClientFile(cfilepath,1); //Puts the file out in the
directory
zptr = NULL;
This works just fine for me in release and debug mode but my user gets
a gpf as soon as he enters into the function.
Am I doing something incorrect? Not allocating something that should
be allocated? Any ideas?
Thanks Josh
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
joshturner1967
|
2/3/2009 12:10:48 AM |
|
joshturner1967 wrote:
> I have the following c++ code
>
> char **filename is passed and is allocated in the calling function as
> char *filename = malloc(1000);
[...]
> tptr = strrchr(mimetype, '/');
[...]
> sprintf(randomintegerchar, "%d.", randominteger);
[...]
> Am I doing something incorrect?
You are not using std::string, which you should do unless there are good
reasons not to do so.
Uli
--
Sator Laser GmbH
Gesch�ftsf�hrer: Thorsten F�cking, Amtsgericht Hamburg HR B62 932
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
Ulrich
|
2/3/2009 9:09:57 AM
|
|
On Feb 3, 6:10 am, joshturner1967 <joshturner1...@gmail.com> wrote:
> I have the following c++ code
> strcat(cfilepath,"\\");
> strcat(cfilepath, *filename);
The pointer cfilepath is not initialised before you do
the strcat().
> Am I doing something incorrect?
Apart from the above, the main problems are:
- using C idioms & features rather than C++
- use of magic numbers
- not checking return values (e.g. that of strrchr() )
- not initialising variables at the point of definition
(leading to your specific problem)
- not using the C++ std::string class
I suggest investigating addressing the last of these
will lead you to solutions to most of the other problems.
Neil Butterworth
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
Neil
|
2/3/2009 9:10:45 AM
|
|
It's hard to tell without knowing more about the context, but strcat
(qfilename, tptr+1); line is almost certainly bad.
Without tiptoeing around, the rest looks quite bad to me, too.
You are most likely overwriting memory you don't own, so debug vs
release does not matter.
If you could outline the complete function, specify what are inputs/
outputs, and explain what getMimeType does (most importantly, where
does it's return value come from), one could say more.
Other than that, you could spare yourself the trouble and work with
std::string or some other string implementation. Forget pure C for
simple
HTH,
Goran.
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
Goran
|
2/3/2009 9:12:58 AM
|
|
On 3 f�v, 07:10, joshturner1967 <joshturner1...@gmail.com> wrote:
> I have the following c++ code
>
> [...]
> This works just fine for me in release and debug mode but my user gets
> a gpf as soon as he enters into the function.
>
> Am I doing something incorrect? Not allocating something that should
> be allocated? Any ideas?
Use valgrind and you'll see.
By the way, it is recommended you either use C or C++, not C-ish C++.
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
Mathias
|
2/3/2009 9:13:22 AM
|
|
On Feb 3, 1:10 am, joshturner1967 <joshturner1...@gmail.com> wrote:
> I have the following c++ code
>
> char **filename is passed and is allocated in the calling function as
> char *filename = malloc(1000);
>
> int randominteger;
> char *zptr;
> char qfilename[500];
> char randomintegerchar[500];
> char *mimetype;
> char *tptr;
> char *cfilepath;
>
> mimetype = part->getMimeType(); //Get the file type
> tptr = strrchr(mimetype, '/'); //Strip off the extra values from the
> getmimetype call
> strcpy(qfilename, "BaseTempFile"); //Start building the temp
> filename
> randominteger = rand(); ///Get a random number
> sprintf(randomintegerchar, "%d.", randominteger); //Turn the number
> into a string
>
> //Build the filename
> strcat(qfilename, randomintegerchar); //Add the random number to make
> the filename to make it unique
> strcat(qfilename, tptr+1);
> zptr = qfilename;
> strcpy(*filename, zptr); //passed back to update the import name
>
> strcat(cfilepath,"\\");
> strcat(cfilepath, *filename);
> part->getContentToClientFile(cfilepath,1); //Puts the file out in the
> directory
> zptr = NULL;
>
> This works just fine for me in release and debug mode but my user gets
> a gpf as soon as he enters into the function.
>
> Am I doing something incorrect? Not allocating something that should
> be allocated? Any ideas?
>
> Thanks Josh
I appreciate the replies
I know this sounds lame but I am mainly from the c world and don't
venture into the c++ realm to often. That is why I was using the
strcat and strcpy functions instead of std. I am not as famliar with
std but what I gather I can do the following
namespace std;
string qfilename;
string mimetype;
string cfilepath = "C:\\Value\\import";
string basetempfile = "basetempfile";
string themime;
string randomintegerchar;
int randominteger;
qfilename.append(basetempfile);
mimetype = part->getMimeType(); //Mimetype would be in format of image/
tif and I would need tif
themime = mimetype.find_last_of("/"); <----Would I use this function
call to extract tif out of mimetype?
randominteger = rand(); <-----Get a random number
itoa(randominteger, randomintegerchar, 10); <----Put random number to
a string
qfilename.copy(basetempfile); <--add the basename
qfilename.append(randomintegerchar);<---add the random number
qfilename.append(".");<----Add the period
qfilename.append(themime);-<Add the filetype
cfilepath.append("\\");
cfilepath.append(qfilename);-<----Add the filepath to the directory
part->getContentToClientFile(cfilepath,1); //Puts the file out in the
directory
First off does this look better or am I off track. If I am off track
could you tell me where and show me what I should be doing?
Secondly I would like to be able to update filename as well char
**filename from qfilename but using this new method I am confussed and
I dont want to switch between c and c++ and want to learn it
correctly.
I know this is second nature to you guys but I am still in the
learning phase and would appreciate your expert advice so as to be as
good as you guys one day
Thanks
Josh
qfilename.append(basetempfile);
mimetype = part->getMimeType();
{ Edits: quoted banner removed. -mod }
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
joshturner1967
|
2/4/2009 12:40:52 AM
|
|
On Feb 3, 11:40 pm, joshturner1967 <joshturner1...@gmail.com> wrote:
> I appreciate the replies
>
> I know this sounds lame but I am mainly from the c world and don't
> venture into the c++ realm to often. That is why I was using the
> strcat and strcpy functions instead of std. I am not as famliar with
> std but what I gather I can do the following
You sound far better than people who just blatantly refuse to use C++
features. If you are making an honest effort to learn, you've already
separated yourself from the herd.
> namespace std;
The directive would be expressed as "using namespace std;", but this
is considered bad practice for reasons outlined at
http://www.parashift.com/c++-faq-lite/coding-standards.html#faq-27.5
(that whole website should be mandatory reading for any C++
programmer)
> string qfilename;
> string mimetype;
> string cfilepath = "C:\\Value\\import";
> string basetempfile = "basetempfile";
> string themime;
> string randomintegerchar;
> int randominteger;
In C++ and in modern C, it is not necessary to declare all your
variables at the start of the function, and in C++ it is generally
considered good practice to initialize variables as you use them, so
as to limit the scope and make it more clear what the purpose of the
variable is.
> qfilename.append(basetempfile);
qfilename.append(basetempfile); can also be written qfilename +=
basetempfile; which you use is a matter of style. I tend to prefer the
latter. In addition, since this is the first use of qfilename, it
would be better to simply have declared it as "string qfilename =
basetempfile;"
> mimetype = part->getMimeType(); //Mimetype would be in format of image/
> tif and I would need tif
>
> themime = mimetype.find_last_of("/"); <----Would I use this function
> call to extract tif out of mimetype?
No. find_last_of() gives you the index into the string of the last
occurrence of a character. You would want themime = mimetype.substr
(mimetype.find_last_of("/") + 1, string::npos); The npos parameter is
optional, but I included it so that you can understand better; substr
will return the substring in between those two indices, and when used
as an index, string::npos means "the end of the string". This code has
another bug in it, which is that if mimetype has no / in it, it will
itself return npos and, since npos is usually an unsigned -1, will
become 0, meaning themime becomes the whole string. You can solve this
by checking for npos in your return value.
> randominteger = rand(); <-----Get a random number
If you are going to use rand(), make sure you seed it first (it's the
same as in C).
> itoa(randominteger, randomintegerchar, 10); <----Put random number to
> a string
Try using one of the methods suggested at
http://www.parashift.com/c++-faq-lite/misc-technical-issues.html#faq-39.1,
or look into Boost.LexicalCast, which is a useful wrapper around the
stringstream libraries.
> qfilename.copy(basetempfile); <--add the basename
You alread assigned qfilename to basetempfile;
> qfilename.append(randomintegerchar);<---add the random number
> qfilename.append(".");<----Add the period
> qfilename.append(themime);-<Add the filetype
You can write this as a single statement:
qfilename.append(randomintegerchar).append(".").append(themime);
-or-
qfilename += randomintegerchar += "." += themime;
> cfilepath.append("\\");
> cfilepath.append(qfilename);-<----Add the filepath to the directory
>
> part->getContentToClientFile(cfilepath,1); //Puts the file out in the
> directory
If getContentToClientFile takes a const char*, you will need to use
cfilepath.c_srt() to get the C string representation.
> First off does this look better or am I off track. If I am off track
> could you tell me where and show me what I should be doing?
>
> Secondly I would like to be able to update filename as well char
> **filename from qfilename but using this new method I am confussed and
> I dont want to switch between c and c++ and want to learn it
> correctly.
Rather than pass a pointer to char*, you should pass a reference to
string. Then you can simply assign filename to the variable in
question:
void foo(string& s)
{
s = "bar";
}
Now, foo will change the value of whatever string is passed to "bar",
without needing any special notation other than in the parameter list.
> I know this is second nature to you guys but I am still in the
> learning phase and would appreciate your expert advice so as to be as
> good as you guys one day
>
> Thanks
> Josh
Don't worry about it. Everyone was at your stage at some point.
Sean
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
Sean
|
2/4/2009 6:37:01 AM
|
|
On Feb 4, 6:40 am, joshturner1967 <joshturner1...@gmail.com> wrote:
> On Feb 3, 1:10 am, joshturner1967 <joshturner1...@gmail.com> wrote:
>
> I gather I can do the following
[code snipped]
> First off does this look better or am I off track. If I am off track
> could you tell me where and show me what I should be doing?
Definitely on track. You may want to look at the following code which
is a solution to your problem (as I understand it) using fairly
idiomatic C++:
#include <string>
#include <iostream>
#include <sstream>
#include <cstdlib>
using namespace std;
// handy function to turn any streamable type into a string
template <typename T> string ToString( const T & t ) {
ostringstream os;
os << t;
return os.str();
}
// build filename from base, random number and mimetype extension
string MakeFileName( const string & base, const string & mime ) {
string filename = base; // always try to initialse
// find the position of the last slash & extract type
// if there isn't one, use whole mime type
string::size_type pos = mime.find_last_of( "/" );
string ext = pos == string::npos ? mime : mime.substr( pos +
1 );
// get string rep of a random number
string rnum = ToString( rand() );
// build the rest of the filename
// note we can use += as well as append()
filename += rnum;
filename += ".";
filename += ext;
return filename;
}
// test it
int main() {
cout << MakeFileName( "myfile", "text/xml" ) << endl;
return 0;
}
Note that in in te above thre are no fixed sized buffers and no
possibilities of buffer overruns.
> Secondly I would like to be able to update filename as well char
> **filename from qfilename but using this new method I am confussed and
> I dont want to switch between c and c++ and want to learn it
> correctly.
Assuming filename points to something, and assuming you are using
malloc/free in the rest of the code then:
// uncompiled & untested
string f = MakeFileName( "myfile", "text/xml" );
* filename = malloc( sizeof(char) * f.size() + 1 );
strcpy( * filename, f.c_str() );
The string member function c_str() gives you a C-style null-terminated
string.
Neil Butterworth
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
Neil
|
2/4/2009 6:39:27 AM
|
|
Thanks Sean and Neil for replying
This is what I came up with based upon your suggetions. No real error
checking as of yet just wanted to get the basics down before I did
error checking
using namespace std;
class BadConversion : public std::runtime_error {
public:
BadConversion(const std::string& s)
: std::runtime_error(s)
{ }
};
inline std::string stringify(int x)
{
std::ostringstream o;
if (!(o << x))
throw BadConversion("stringify(int)");
return o.str();
}
string basetempfile = "basetempfile";
string themime;
string randomintegerchar;
string heldfilepath;
int randominteger;
srand(67); //Seed rand()
npos = -1;
qfilename.append("BaseTempFile"); //Start building the temp filename
mimetype = part->getMimeType(); //Get the file type
themime = mimetype.substr(mimetype.find_last_of("/") + 1, npos); ///
Strip off the file type
randominteger = rand(); ///Get the random number
randomintegerchar = stringify(randominteger); //Turn it into a string
qfilename.append(randomintegerchar).append(".").append(themime); ///
Put the values together to form the file name
strcpy( * filename, qfilename.c_str() ); ////gives the C-style null-
terminated string.
heldfilepath.append("\\");
heldfilepath.append(qfilename);
strcat( cfilepath, heldfilepath.c_str() );
part->getContentToClientFile(cfilepath,1); //Puts the file out in the
directory
heldfilepath.clear();
qfilename.clear();
This does not gpf for me and puts the files to the directory - I plan
on testing with my user but is there anything else that I missed?
Thanks
Josh
FYI I did not include any of the orignal comments to prevent rejection
from the moderator in regard to
{ Please trim the quoting, and please don't quote signatures or the clc
++m
banner. -mod }
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
0
|
|
|
|
Reply
|
joshturner1967
|
2/6/2009 2:19:19 AM
|
|
|
8 Replies
96 Views
(page loaded in 0.127 seconds)
|