what's the problem with the snippet??

[eternity007]

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.

#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

[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?

[eternity007]

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.

[monarch_dodra]

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".

[eternity007]

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.

[Paul McKenzie]

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

[eternity007]

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.

[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.

[eternity007]

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.

[GCDEF]

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.

[eternity007]

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.

[laserlight]

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.

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.

[monarch_dodra]

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.

[GCDEF]

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?

[eternity007]

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.