-
April 26th, 2012, 01:37 PM
#1
what's the problem with the snippet??
Hi,
I have function which will replace (or create) an file with the contents of another stream.
The stream could be anything. The replacement is done safely.
The code might have multiple potential problems. Can anyone help me to find out.
Code:
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#include <unistd.h>
int do_replace(const char *file, int stream, int cnt)
{
char *env = getenv("CDIR");
if (env == NULL)
env = ".";
char *rpath = realpath(env, NULL);
if (rpath == NULL || strlen(rpath) >= PATH_MAX) {
free(rpath);
return -1;
}
char tname[PATH_MAX+32];
snprintf(tname, sizeof tname, "%s/safetemp_XXXXXX", rpath);
free(rpath);
int ofd = mkstemp(tname);
if (ofd == -1)
return -2;
char rbuf[1024], *pbuf, bad = 0;
while (!bad) {
int r = read(stream, pbuf=rbuf, sizeof rbuf);
if (r == 0) break;
else if (r < 0 && EINTR == errno) continue;
else if (r < 0) bad = 1;
else while (r > 0 && !bad) {
int w = write(ofd, pbuf, r);
if (w > 0) {
r -= w;
pbuf += w;
} else bad = 1;
}
}
if (close(ofd)) bad = 1;
if (close(stream)) bad = 1;
if (cnt == 1)
sleep(100);
if (!bad && rename(tname, file))
bad = 1;
if (bad) {
unlink(tname);
return -3;
}
return 0;
}
Thanks in advance
-
April 26th, 2012, 01:48 PM
#2
Re: what's the problem with the snippet??
Is this homework or a job interview question? Why do you think it might have problems and what do you think they are, other than horrible formatting and meaningless variable names?
-
April 27th, 2012, 12:56 AM
#3
Re: what's the problem with the snippet??
Originally Posted by GCDEF
Is this homework or a job interview question? Why do you think it might have problems and what do you think they are, other than horrible formatting and meaningless variable names?
Its a question asked to me by one of my friend. Not sure where did he get it.
Being a programmer, we know formatting and variable naming can not be a problem with code. Though its irritating.
It might have some problem(s) cleverly hidden within it.
-
April 27th, 2012, 03:36 AM
#4
Re: what's the problem with the snippet??
Originally Posted by eternity007
It might have some problem(s) cleverly hidden within it.
In a word, that's the problem. More important than not having problems, is if you can trust the function to not have a problem.
You could re-read this thing 20 times, and debug it a few times, and you still wouldn't be 100% sure its safe.
Good code you can just skim through, and it leaves no room for "might have cleverly hidden problems".
Is your question related to IO?
Read this C++ FAQ article at parashift by Marshall Cline. In particular points 1-6.
It will explain how to correctly deal with IO, how to validate input, and why you shouldn't count on "while(!in.eof())". And it always makes for excellent reading.
-
April 27th, 2012, 04:04 AM
#5
Re: what's the problem with the snippet??
Originally Posted by monarch_dodra
In a word, that's the problem. More important than not having problems, is if you can trust the function to not have a problem.
You could re-read this thing 20 times, and debug it a few times, and you still wouldn't be 100% sure its safe.
Good code you can just skim through, and it leaves no room for "might have cleverly hidden problems".
I never thought the discussion will go in this way. I was more keen to know if anyone can see any potential problem with this code or not, on system perspective rather on simple code perspective.
For example, the function closes the stream variable which it doesn't own. This could lead to a potential problem while running the code as the owner might not expect it to be closed by someone else.
Anyways, its my mistake to post a horribly wrongly formatted code here.
And thanks for your replies.
Last edited by eternity007; April 27th, 2012 at 04:14 AM.
-
April 27th, 2012, 04:42 AM
#6
Re: what's the problem with the snippet??
Originally Posted by eternity007
For example, the function closes the stream variable which it doesn't own. This could lead to a potential problem while running the code as the owner might not expect it to be closed by someone else.
If that function is documented that the stream will close on success, the problem is the user not reading the documentation.
Regards,
Paul McKenzie
Last edited by Paul McKenzie; April 27th, 2012 at 04:44 AM.
-
April 27th, 2012, 05:08 AM
#7
Re: what's the problem with the snippet??
Originally Posted by Paul McKenzie
If that function is documented that the stream will close on success, the problem is the user not reading the documentation.
Regards,
Paul McKenzie
As you assumed it might get documented, then it can also be assumed the if not then it's a code problem.
Also, here code is transparent to us and not the documentation.
-
April 27th, 2012, 07:12 AM
#8
Re: what's the problem with the snippet??
The reason you're not getting much help is first that it sounds like homework or a job interview question, second it's really up to you to find the problem, and if you're unsure how to fix it, ask for help. I don't think many of us here want to do your "friend's" work for him/her, by reading through a function whose purpose isn't clear, whose problem is unknown, with meaningless variable names and horrible formatting for the friend of somebody who didn't write it.
-
April 27th, 2012, 08:48 AM
#9
Re: what's the problem with the snippet??
Originally Posted by GCDEF
The reason you're not getting much help is first that it sounds like homework or a job interview question, second it's really up to you to find the problem, and if you're unsure how to fix it, ask for help. I don't think many of us here want to do your "friend's" work for him/her, by reading through a function whose purpose isn't clear, whose problem is unknown, with meaningless variable names and horrible formatting for the friend of somebody who didn't write it.
I tought ppl over here are more interested on technical discussion rather finding out who why when. Also I thought I asked for help to find the problem than simply arguing some unnecessary topics.
Someone keen to help, will just help instead of moving the discussion in unwanted path. And one has right to not to post when not interested to help.
-
April 27th, 2012, 09:04 AM
#10
Re: what's the problem with the snippet??
Originally Posted by eternity007
I tought ppl over here are more interested on technical discussion rather finding out who why when. Also I thought I asked for help to find the problem than simply arguing some unnecessary topics.
Someone keen to help, will just help instead of moving the discussion in unwanted path. And one has right to not to post when not interested to help.
We're interested in helping people who want to help themselves. We're not interested in people who just dump homework or interview questions here and expect us to do their work for them.
-
April 27th, 2012, 11:22 AM
#11
Re: what's the problem with the snippet??
Originally Posted by GCDEF
We're interested in helping people who want to help themselves. We're not interested in people who just dump homework or interview questions here and expect us to do their work for them.
Or you can say it like this way that when u don't understand any question you start pointing to dump things as you have your oath to post something.
-
April 27th, 2012, 12:15 PM
#12
Re: what's the problem with the snippet??
Originally Posted by eternity007
Or you can say it like this way that when u don't understand any question you start pointing to dump things as you have your oath to post something.
Looking at your initial post and your responses, I think it is more likely that you are just dumping a homework/interview/quiz question. If you had shown the effort to answer the question yourself, then you may have gotten a more favourable response.
Originally Posted by eternity007
Someone keen to help, will just help instead of moving the discussion in unwanted path. And one has right to not to post when not interested to help.
Someone keen to get help will make an effort to help themselves instead of fishing to be spoonfed for answers. And one has the right to challenge that someone to make an effort.
-
April 27th, 2012, 12:25 PM
#13
Re: what's the problem with the snippet??
Originally Posted by eternity007
I tought ppl over here are more interested on technical discussion rather finding out who why when.
I gave you an honest answer, as did others, and if this was for an interview, then all the the answer you have gotten were actually good answers to give to an interviewer.
We're all interested in having a technical discussion, but you didn't start one. If you provide something worth discussing, we'll jump aboard.
Is your question related to IO?
Read this C++ FAQ article at parashift by Marshall Cline. In particular points 1-6.
It will explain how to correctly deal with IO, how to validate input, and why you shouldn't count on "while(!in.eof())". And it always makes for excellent reading.
-
April 27th, 2012, 12:34 PM
#14
Re: what's the problem with the snippet??
Originally Posted by eternity007
And one has right to not to post when not interested to help.
I was helping you. I told you a bit about the way the forum works and why you weren't going to get the answer you want by asking the way you were asking. Would you prefer people say nothing, or try to steer you in a direction that will allow you to get help?
-
April 27th, 2012, 12:44 PM
#15
Re: what's the problem with the snippet??
Originally Posted by monarch_dodra
I gave you an honest answer, as did others, and if this was for an interview, then all the the answer you have gotten were actually good answers to give to an interviewer.
We're all interested in having a technical discussion, but you didn't start one. If you provide something worth discussing, we'll jump aboard.
Thanks a lot for your honest answer and so too others.
I submitted something where I have given my effort and want to discuss with pros to evaluate myself ( if you want to believe that).
I have given my effort: that's why I gave the one solution (in one of the previous post) among what I found.
It was pretty simple and straight forward to post a question I had, but doubtful minds are more into finding why it has been posted than discussing it technically. I am assuming you always follow this rule and judge people so easily.
To prove myself, what I can say is better answer this after 15days or a month or even a year if you really able to find something TECHNICALLY. Hopefully by that time no exam or interview will be waiting for the same answer from me.
And if you can not, please stop judging me and say "I could not" also if you find it not worth to discuss you have right to be silent (I said it again) rather poking or discouraging others.
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|