Re: [rdesktop-devel] Saving and Returning TSCAL's

[Mrs. B. <mrs...@ni...>]

On Thu, 2002-11-07 at 11:44, Peter Fales wrote:
> See >>> for my comments.
> 
> Pete
> 
> 
> On Thu, Nov 07, 2002 at 10:42:54AM -0500, Mrs. Brisby wrote:
> > 1. NFS and crash-safety. I think one of the most dangerous things is
> > O_WRONLY|O_TRUNC - it's worthless on filesystems like amoeba, and
> > dangerous when using NFS. Plus, your system crashes, and your files are
> > pooped and in a dangerous state. Never use O_WRONLY|O_TRUNC just because
> > you don't want to type 10 more lines of code.
> 
> >>> Makes sense, though in this case it doesn't seem to serious.  If
> >>> the file is corrupted, the terminal server will simply send a 
> >>> new license blob when rdesktop is nex started,  and it will get written 
> >>> at that time.

Are we certain of this? I don't have the ability to read MS's Terminal
Services code. Does someone with their SS NDA signed know this for
certain?

Anyway, what's the point in wasting a license (or two, or three) if you
launch rdesktop from two machines (sharing an NFS home) during that
race?


> > 
> > Change this around (the end):
> > 	path = malloc(strlen(home)+37);
> > 	if (!path) {
> > 		perror("malloc");
> > 		exit(1);
> > 	}
> > 	q = malloc(strlen(home)+25);
> > 	if (!q) {
> > 		perror("malloc");
> > 		exit(1);
> > 	}
> > 	sprintf(path, "%s/.rdesktop", path);
> 
> >>> path is undefined here.  Do you mean home?
> >>> Would it make sense to add some string variables for "/.rdesktop" and
> >>> "/license" (or "/licence") so that can use strlen instead of 
> >>> hardcoding 25 and 37?

whoops, no I meant "home"; not path. It's late for me.... Thanks for
catching it.

> 
> > 	if (mkdir(path, 0700) == -1 && errno != EEXIST) {
> > 		perror("mkdir");
> > 		exit(1);
> > 	}
> > 	sprintf(q, "%s/license", path);
> > 	ofd = open(q, O_RDONLY);
> > 	if (ofd != -1) {
> > 		fl.l_type = F_WRLCK;
> > 		fl.l_whence = SEEK_SET;
> > 		fl.l_start = 0;
> > 		fl.l_len = 1;
> > 		fcntl(ofd, F_SETLK, &fl);
> > 	}
> > 
> > 	x = strchr(path, '\0');
> > 	for (y = 0;; y++) {
> > 		sprintf(x, "/license.%lu", y);
> > 		fd = open(path, O_WRONLY|O_CREAT|O_EXCL, 0600);
> > 		if (fd == -1) {
> > 			if (errno == EINTR || errno == EEXIST) continue;
> > 			perror("creat");
> > 			exit(1);
> > 		}
> > 		break;
> > 	}
> > 	for (y = 0; y < len;) {
> > 		do {
> > 			r = write(fd, s_ptr->p+y, len-y);
> > 		} while (r == -1 && errno == EINTR);
> > 		if (r < 1) {
> > 			perror("write");
> > 			unlink(path);
> > 			exit(1);
> > 		}
> > 		y += r;
> > 	}
> > 	if (close(fd) == -1) {
> > 		perror("close");
> > 		unlink(path);
> > 		exit(1);
> > 	}
> > 	if (rename(path, q) == -1) {
> > 		perror("rename");
> > 		unlink(path);
> > 		exit(1);
> > 	}
> > 	if (ofd != -1) {
> > 		fl.l_type = F_UNLCK;
> > 		fl.l_whence = SEEK_SET;
> > 		fl.l_start = 0;
> > 		fl.l_len = 1;
> > 		fcntl(ofd, F_SETLK, &fl);
> > 		close(ofd);
> > 	}
> >