On line 193 of "lib/xfile.c" there is a call to x_fflush which may start an infinite looping if something goes wrong during the flush and before f->bufused is updated. Quick fix: test the return value of x_fflush and act accordingly Better fix: replace the loop ; I've drafted some code that I can mail you if you're interested Note: this may explain the loop reported in comment #14 of bug #3817 Note: I've assigned this bug to 'ntlm_auth' because in ntlm_auth.c there is a lot of indirect calls to x_fwrite (through x_fprintf). But of course any other component of samba may be impacted.
Created attachment 3187 [details] non-looping draft implementation of x_fwrite This is draft code I've written to implement x_fwrite with no loop. Please note that this code has not been tested, not even compiled. Note also that this implementation better emulates fwrite in that upon error it returns zero or a positive number smaller than nmemb. Besides, the prototype of x_fwrite forbids the returning of -1.
Comment on attachment 3187 [details] non-looping draft implementation of x_fwrite ><html><body><pre style="word-wrap: break-word; white-space: pre-wrap;">/* simulate fwrite() */ >size_t x_fwrite(const void *p, size_t size, size_t nmemb, XFILE *f) >{ > ssize_t ret; > size_t total=size*nmemb; // we should check for overflow... > > if (!total) return 0; // this is what fwrite must do according to the standard > > /* we might be writing unbuffered */ > if (f->buftype == X_IONBF || > (!f->buf && !x_allocate_buffer(f))) { > ret = write(f->fd, p, total); > if (ret < 0) return 0; > return ret/size; > } > > if (total <= f->bufsize - f->bufused) { > memcpy(f->buf + f->bufused, (const char *)p, total); > f->bufused += total; > } else { > size_t buffered = total % f->bufsize; // 0 <= buffered <= total > if (x_fflush(f) < 0) return 0; // flush old content before writing more bytes > ret = write(f->fd, (const char *)p, total-buffered); // no-op when (buffered == total); write all when (buffered == 0) > if (ret < 0) return 0; > if (ret < total-buffered) return ret/size; > memcpy(f->buf, total-buffered+(const char *)p, buffered); // no-op when (buffered == 0); copy all when (buffered == total) > f->bufused = buffered; > } > } > /* when line buffered we need to flush at the last linefeed. This can > flush a bit more than necessary, but that is harmless */ > if (f->buftype == X_IOLBF && f->bufused) { > int i; > for (i=f->bufused-1; i>=0; i--) { > if (*(i+f->buf) == '\n') { > if (x_fflush(f) < 0) return (total-f->bufused)/size; > break; > } > } > } > > return nmemb; >}
Comment on attachment 3187 [details] non-looping draft implementation of x_fwrite In the code attached to comment#2 > if (ret < total-buffered) return written/size; should be if (ret < total-buffered) return ret/size;
It would be best if you added a patch from "git format-patch". This way you would also get all the credit in the author information. Thanks, Volker
Created attachment 3243 [details] Patch I think this is a simpler patch for the problem. Not as clean as a rewrite, but easier to get into the current codebase. Let me know what you think. Jeremy.
The patch is not in git am format. I will upload a correct patch next. Volker
Created attachment 5612 [details] git am format patch Patch in git am format against master
xfile.c is gone :-)