Re: Safe file update library ready (sort of)
On 06/02/11 18:40, Goswin von Brederlow wrote: I absolutely hate that. A header file should be compilable on its own. The times when #includefoo.h would slow down the compiler are long gone and all include files are protected with #ifndef NAME so duplicate includes are harmless. On the other hand finding out what include files to include and in what order is a real pain if you have multiple files. Even if you have read the manual you will have to reread it every time you start a new project again and again to get that right. The includes necessary for safe_open are those necessary for open, plus safe_write.h itself. I don't see that as particularly burdensome. I'm not sure how platform independent any includes I put inside my header are going to be, and would rather not open this particular can of worms. Shachar -- Shachar Shemesh Lingnu Open Source Consulting Ltd. http://www.lingnu.com -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4d51bcb3.7010...@debian.org
Re: Safe file update library ready (sort of)
Shachar Shemesh shac...@debian.org writes:
On 26/01/11 13:03, Goswin von Brederlow wrote:
Some things I noticed:
safewrite.h:
- missing headers, e.g. for mode_t
No. That's intentional. I'm assuming the people who will use
safewrite.h are going to RTFM, where it clearly says that those
includes are needed. I might reconsider if valid reasons are provided,
but I would like to avoid keep including the same headers over and
over.
I absolutely hate that. A header file should be compilable on its
own. The times when #include foo.h would slow down the compiler are
long gone and all include files are protected with #ifndef NAME so
duplicate includes are harmless.
On the other hand finding out what include files to include and in what
order is a real pain if you have multiple files. Even if you have read
the manual you will have to reread it every time you start a new project
again and again to get that right.
- no 'extern C {'
You are right. Fixed now.
I don't like how your functions are destructive to the path argument.
I don't think that is a major issue, but I do think that the reliance
on PATH_MAX is. I think the current implementation solves both of
these concerns.
Shachar
Well, PATH_MAX breaks on hurd iirc so that is (or was) a reall show stopped.
MfG
Goswin
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/87ei7lt8u9.fsf@frosties.localnet
Re: Safe file update library ready (sort of)
On 26/01/11 13:03, Goswin von Brederlow wrote:
Some things I noticed:
safewrite.h:
- missing headers, e.g. for mode_t
No. That's intentional. I'm assuming the people who will use safewrite.h
are going to RTFM, where it clearly says that those includes are needed.
I might reconsider if valid reasons are provided, but I would like to
avoid keep including the same headers over and over.
- no 'extern C {'
You are right. Fixed now.
I don't like how your functions are destructive to the path argument.
I don't think that is a major issue, but I do think that the reliance on
PATH_MAX is. I think the current implementation solves both of these
concerns.
Shachar
--
Shachar Shemesh
Lingnu Open Source Consulting Ltd.
http://www.lingnu.com
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4d4d984b.9070...@debian.org
Re: Safe file update library ready (sort of)
On Fri, Jan 28, 2011 at 07:37:02AM +0100, Olaf van der Spek wrote: Is there a way to log cases where (potentially) unsafe writes happen? Cases like truncation of an existing file, rename on a target that's not yet synced, etc. Not really, because there are plenty of cases where it's perfectly OK not to sync a file on close or on rename. Any files created during a build, for example, can easily be reproduced in the unlikely case of a systme crash. If you are untaring a source tree, it's fine not to worry about syncing out the files, since presumably you can always repeat the untar operation. Or take git; when git is checking out files into the working directory, there's no reason that has to be done in a super-safe way. On the other hand, when it is writing the git object files and pack files, those had better be done safely. At the end of the day the application programmer needs to understand what is going on, and write his code appropriately based on the needs of his application with respect to reliability after a power crash. So how can you just log warnings that the program has just done something unsafe? It's unsafe only if there's no other way to reconstitute the data that was just written. But that's not something which is easily knowable. (I know, I'm being *so* unfair; I'm expecting application programmers to be competent...) - Ted -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110129125308.ga19...@thunk.org
Re: Safe file update library ready (sort of)
On Sat, Jan 29, 2011 at 1:53 PM, Ted Ts'o ty...@mit.edu wrote: On Fri, Jan 28, 2011 at 07:37:02AM +0100, Olaf van der Spek wrote: Is there a way to log cases where (potentially) unsafe writes happen? Cases like truncation of an existing file, rename on a target that's not yet synced, etc. Not really, because there are plenty of cases where it's perfectly OK not to sync a file on close or on rename. Any files created during a build, for example, can easily be reproduced in the unlikely case of a systme crash. If you are untaring a source tree, it's fine not to worry about syncing out the files, since presumably you can always repeat the untar operation. Or take git; when git is checking out files into the working directory, there's no reason that has to be done in a super-safe way. On the other hand, when it is writing the git object files and pack files, those had better be done safely. At the end of the day the application programmer needs to understand what is going on, and write his code appropriately based on the needs of his application with respect to reliability after a power crash. So how can you just log warnings that the program has just done something unsafe? It's unsafe only if there's no other way to reconstitute the data that was just written. But that's not something which is easily knowable. If all potentially unsafe cases could be logged, the log can be analyzed by a human to detect the real unsafe cases such that the developers of those apps can be notified and the apps can be fixed. -- Olaf -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/aanlktimbiodsp4qndcozcnp1yr_6rnxjmw7xtdvgs...@mail.gmail.com
Re: Safe file update library ready (sort of)
Hendrik Sattler p...@hendrik-sattler.de writes: char buffer[0]; is veeery gcc-specific as the storage size of buffer is 0. According to the C99 standard: 6.7.5.2 Array declarators Constraints 1 In addition to optional type qualifiers and the keyword static, the [ and ] may delimit an expression or *. If they delimit an expression (which specifies the size of an array), the expression shall have an integer type. If the expression is a constant expression, it shall have a value greater than zero. Iirc the [0] is the pre C99 syntax that behaves the same as [] in sane compilers and still works in compilers that didn't support []. You would get a few extra byte allocated in that case. But yes. Best to do it right and screw the non C99 compilers. :) Either make this char buffer[1]; and live with the fact that e.g. cppcheck will yell at you (better not), or use safe_t x= ...; char *buffer = x + 1; with or without explicit reference in safe_t (if you want to allocate in one block) or simply allocate it seperately. Uh oh, no pointer arithmetic there. C99 does have the unspecified size arrays at the end of a struct so that one can specify texactly this use case. BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! HS And the error is in something generic so every KDE app uses the same horrible thing. :) MfG Goswin -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/874o8raghy.fsf@frosties.localnet
Re: Safe file update library ready (sort of)
On Friday 28 January 2011 03:26:42 Ted Ts'o wrote: On Wed, Jan 26, 2011 at 06:14:42PM +0100, Olaf van der Spek wrote: On Wed, Jan 26, 2011 at 5:36 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! Well kde4 has a special ability for loosing configurations, i can't see any reason because a failed s2ram should destroy configurations that were not in editing-phase since months. As workaround i suggest alias s2ram=sync;s2ram Now that i think of it. We could force a sync before suspending. Maybe patching s2ram to sync() before taking any action? This wouldn't solve the problem but could help making the cases when it occurs more rare. And anyway the filesystem would be in a better state when it is fscked. If you don't like that answer, you'll find that it's true for any other OS (i.e., BSD, OpenSolaris, etc.) --- so either KDE needs to get with the program, or find its users gradually switching to other windowing systems that have sanely written libraries. I agree here, but kde guys are hard to convince that something needs to be changed... Bye -- Salvo Tomaselli signature.asc Description: This is a digitally signed message part.
Re: Safe file update library ready (sort of)
On 2011-01-26 17:36:19 +0100, Hendrik Sattler wrote:
Zitat von Goswin von Brederlow goswin-...@web.de:
Hendrik Sattler p...@hendrik-sattler.de writes:
Zitat von Goswin von Brederlow goswin-...@web.de:
typedef struct {
int fd;
char buffer[0];
} safe_t;
and allocating the struct as big as needed.
Maybe don't recommend invalid C? Bad habits don't have to live on forever...
HS
Would you use
typedef struct {
int fd;
char buffer[];
} safe_t;
or what do you mean by invalid C?
char buffer[0]; is veeery gcc-specific as the storage size of
buffer is 0. According to the C99 standard:
6.7.5.2 Array declarators
Constraints
1 In addition to optional type qualifiers and the keyword static, the [ and
] may delimit an expression or *. If they delimit an expression (which
specifies the size of an array), the expression shall have an integer type.
If the expression is a constant expression, it shall have a value greater
than zero.
As the intended code seems to be a flexible array member wouldn't
§6.7.2.1 (16) apply here?
See also http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Either make this char buffer[1]; and live with the fact that e.g.
cppcheck will yell at you (better not), or use safe_t x= ...; char
*buffer = x + 1; with or without explicit reference in safe_t (if
you want to allocate in one block) or simply allocate it seperately.
Depending on how this buffer is used this might cause compilation
problems when -D_FORTIFY_SOURCE_ is used as gcc checks for strcpy() the
size of the destination buffer and complains if it's too small.
Regards,
Michael
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110127174720.ga8...@vorlon.ping.de
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 06:14:42PM +0100, Olaf van der Spek wrote: On Wed, Jan 26, 2011 at 5:36 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! Maybe complain to the Linux kernel people instead. It won't be just XFS or ext4, but any file system except ext3 (which has performance problems specifically *because* of an implementation detail accidentally provided this feature you like), and I think what you'll find is that most Linux kernel developers will tell you is that it's a bug in the application. If you don't like that answer, you'll find that it's true for any other OS (i.e., BSD, OpenSolaris, etc.) --- so either KDE needs to get with the program, or find its users gradually switching to other windowing systems that have sanely written libraries. - Ted P.S. There is a kernel options that provide improved ext3 performance, to wit, CONFIG_EXT3_DEFAULTS_TO_ORDERED=no, which will also mean that you had better use fsync() if you want files pushed out to disk. So strictly speaking, it's not even true that KDE4 is guaranteed to be safe if you use ext3. -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110128022642.ga15...@thunk.org
Re: Safe file update library ready (sort of)
On Fri, Jan 28, 2011 at 3:26 AM, Ted Ts'o ty...@mit.edu wrote: On Wed, Jan 26, 2011 at 06:14:42PM +0100, Olaf van der Spek wrote: On Wed, Jan 26, 2011 at 5:36 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! Maybe complain to the Linux kernel people instead. It won't be just XFS or ext4, but any file system except ext3 (which has performance problems specifically *because* of an implementation detail accidentally provided this feature you like), and I think what you'll find is that most Linux kernel developers will tell you is that it's a bug in the application. If you don't like that answer, you'll find that it's true for any other OS (i.e., BSD, OpenSolaris, etc.) --- so either KDE needs to get with the program, or find its users gradually switching to other windowing systems that have sanely written libraries. - Ted P.S. There is a kernel options that provide improved ext3 performance, to wit, CONFIG_EXT3_DEFAULTS_TO_ORDERED=no, which will also mean that you had better use fsync() if you want files pushed out to disk. So strictly speaking, it's not even true that KDE4 is guaranteed to be safe if you use ext3. Is there a way to log cases where (potentially) unsafe writes happen? Cases like truncation of an existing file, rename on a target that's not yet synced, etc. -- Olaf -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/AANLkTikHrfBjaBK5=qkSi3b=kbm1jx15r-f10ijla...@mail.gmail.com
Re: Safe file update library ready (sort of)
Ian Jackson ijack...@chiark.greenend.org.uk writes: Shachar Shemesh writes (Re: Safe file update library ready (sort of)): I'm sorry, it might be me, but I fail to see the overlap between the functionalities of safewrite vs. userv. The premises for safewrite is that a program wants to make sure data integrity is maintained when writing files. Userv seems to be about trust and a user level tool. The two seem to revolve around two completely different interpretations to the word safe, as well as two completely different use scenarios. Am I missing something here? Sorry, I replied to the wrong message. I meant to reply to Adam Borowski's comment, where he wrote: ] There's a race condition: ] ] while [ 1 ]; do ln -s /etc/passwd somefile.tmp; done ] Hey root, could you please use this program using libsafewrite on ] 'somefile'? Having said that, I don't think the concept behind your library is sound, because it presupposes that all previous programs which update files are buggy. Just because some wrongheaded Linux kernel filesystem developers think that almost all previously written Unix programs are buggy, doesn't mean that it's true or that the right fix is to rewrite every program. Ian. I think you are dead wrong there Ian. Even if every single program is dead right (and we know a lot aren't) that means every one of them has a safe file update function somewhere in it. A function doing exactly the same thing in many programms. Doesn't that just scream for a shared library? Add to that the number of programs that don't yet do file updates in a safe way and need to be fixed I think the project is a verry good idea. The unexpected behaviour of ext4 is just a minor implementation detail to take care for a general safe update function. So Shachar don't get discuraged by the ocassional nay sayer. MfG Goswin -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/87r5c06jey.fsf@frosties.localnet
Re: Safe file update library ready (sort of)
Shachar Shemesh shac...@debian.org writes:
Hi all,
I've promised to get a library out there, and here it is. The base URL
is https://github.com/Shachar/safewrite, and the actual code is at
https://github.com/Shachar/safewrite/blob/master/safewrite.c
This is not a formal release just yet (plus one function is still
missing an implementation, trivial though it might be). It's just that
the list obviously has a few people knowledgeable on the subject who
can give my code a second look and see whether there is anything I
have missed.
I'll probably make an official release over the next couple of
days. Feedback most appreciated.
Shachar
Some things I noticed:
safewrite.h:
- missing headers, e.g. for mode_t
- no 'extern C {'
I don't like how your functions are destructive to the path argument. I
get that you need to cerate the real path and return that. But maybe you
could use
tyepdef char * path_t;
int safe_open( const char *name, path_t *path, int flags, mode_t mode )
int safe_close( const path_t *path, int fd )
int safe_close_sync( char path_t *path, int fd)
That way one could use
path_t path;
int fd = safe_open(.myapp.rc, path, ...);
OR
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
safe_t * safe_open( const char *name, int flags, mode_t mode );
int safe_close( const safe_t *object);
int safe_close_sync( safe_t *object) ;
static inline int safe_fd(const safe_t *object) {
if (object == NULL) {
return -1;
} else {
return object-fd;
}
}
Just some thoughts,
Mrvn
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/87mxmo6i5j.fsf@frosties.localnet
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 12:03:52PM +0100, Goswin von Brederlow wrote:
Shachar Shemesh shac...@debian.org writes:
I've promised to get a library out there, and here it is. The base URL
is https://github.com/Shachar/safewrite, and the actual code is at
https://github.com/Shachar/safewrite/blob/master/safewrite.c
Some things I noticed:
That way one could use
[..]
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
Except, you can't rely on PATH_MAX on any modern system. It's defined in
Linux headers to an arbitrary value to make old code compile, but for
example Hurd folks decided to not define it altogether to make buggy code
fail during compilation rather than on runtime.
--
1KB // Microsoft corollary to Hanlon's razor:
// Never attribute to stupidity what can be
// adequately explained by malice.
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110126112229.ga29...@angband.pl
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 12:22 PM, Adam Borowski kilob...@angband.pl wrote:
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
Except, you can't rely on PATH_MAX on any modern system. It's defined in
Linux headers to an arbitrary value to make old code compile, but for
example Hurd folks decided to not define it altogether to make buggy code
fail during compilation rather than on runtime.
That's easily solved by a dynamic allocation.
Olaf
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive:
http://lists.debian.org/aanlktim+oteqsqshz3-fxvg-53cf2iyxsdcxgtsbc...@mail.gmail.com
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 11:36 AM, Goswin von Brederlow goswin-...@web.de wrote: I think you are dead wrong there Ian. Even if every single program is dead right (and we know a lot aren't) that means every one of them has a safe file update function somewhere in it. A function doing exactly the same thing in many programms. Doesn't that just scream for a shared library? If the kernel supported it properly, it'd be even easier and you wouldn't have all regressions. Lacking that, a lib is the next best thing. Add to that the number of programs that don't yet do file updates in a safe way and need to be fixed I think the project is a verry good idea. The unexpected behaviour of ext4 is just a minor implementation detail to take care for a general safe update function. Some instrumentation to detect such unsafe behaviour would be helpful too. Olaf -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/AANLkTimj=78qkXG=xfc3qop5yyydwkpr9v33hx7s_...@mail.gmail.com
Re: Safe file update library ready (sort of)
Adam Borowski kilob...@angband.pl writes:
On Wed, Jan 26, 2011 at 12:03:52PM +0100, Goswin von Brederlow wrote:
Shachar Shemesh shac...@debian.org writes:
I've promised to get a library out there, and here it is. The base URL
is https://github.com/Shachar/safewrite, and the actual code is at
https://github.com/Shachar/safewrite/blob/master/safewrite.c
Some things I noticed:
That way one could use
[..]
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
Except, you can't rely on PATH_MAX on any modern system. It's defined in
Linux headers to an arbitrary value to make old code compile, but for
example Hurd folks decided to not define it altogether to make buggy code
fail during compilation rather than on runtime.
Right, so
typedef struct {
int fd;
char buffer[0];
} safe_t;
and allocating the struct as big as needed.
MfG
Goswin
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/87mxmn6b4u.fsf@frosties.localnet
Re: Safe file update library ready (sort of)
Zitat von Goswin von Brederlow goswin-...@web.de:
Adam Borowski kilob...@angband.pl writes:
On Wed, Jan 26, 2011 at 12:03:52PM +0100, Goswin von Brederlow wrote:
Shachar Shemesh shac...@debian.org writes:
I've promised to get a library out there, and here it is. The base URL
is https://github.com/Shachar/safewrite, and the actual code is at
https://github.com/Shachar/safewrite/blob/master/safewrite.c
Some things I noticed:
That way one could use
[..]
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
Except, you can't rely on PATH_MAX on any modern system. It's defined in
Linux headers to an arbitrary value to make old code compile, but for
example Hurd folks decided to not define it altogether to make buggy code
fail during compilation rather than on runtime.
Right, so
typedef struct {
int fd;
char buffer[0];
} safe_t;
and allocating the struct as big as needed.
Maybe don't recommend invalid C? Bad habits don't have to live on forever...
HS
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110126153957.18702pqz49ugq...@v1539.ncsrv.de
Re: Safe file update library ready (sort of)
Hendrik Sattler p...@hendrik-sattler.de writes:
Zitat von Goswin von Brederlow goswin-...@web.de:
Adam Borowski kilob...@angband.pl writes:
On Wed, Jan 26, 2011 at 12:03:52PM +0100, Goswin von Brederlow wrote:
Shachar Shemesh shac...@debian.org writes:
I've promised to get a library out there, and here it is. The base URL
is https://github.com/Shachar/safewrite, and the actual code is at
https://github.com/Shachar/safewrite/blob/master/safewrite.c
Some things I noticed:
That way one could use
[..]
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
Except, you can't rely on PATH_MAX on any modern system. It's defined in
Linux headers to an arbitrary value to make old code compile, but for
example Hurd folks decided to not define it altogether to make buggy code
fail during compilation rather than on runtime.
Right, so
typedef struct {
int fd;
char buffer[0];
} safe_t;
and allocating the struct as big as needed.
Maybe don't recommend invalid C? Bad habits don't have to live on forever...
HS
Would you use
typedef struct {
int fd;
char buffer[];
} safe_t;
or what do you mean by invalid C?
MfG
Goswin
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/87pqrjmyu8.fsf@frosties.localnet
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 5:09 PM, Goswin von Brederlow goswin-...@web.de wrote:
typedef struct {
int fd;
char buffer[];
} safe_t;
or what do you mean by invalid C?
Zero length arrays are not valid C AFAIK.
--
Olaf
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive:
http://lists.debian.org/aanlktik-gsod2qvvl45ub-ccg_rjb4tk9jg6l+ytu...@mail.gmail.com
Re: Safe file update library ready (sort of)
Zitat von Goswin von Brederlow goswin-...@web.de:
Hendrik Sattler p...@hendrik-sattler.de writes:
Zitat von Goswin von Brederlow goswin-...@web.de:
Adam Borowski kilob...@angband.pl writes:
On Wed, Jan 26, 2011 at 12:03:52PM +0100, Goswin von Brederlow wrote:
Shachar Shemesh shac...@debian.org writes:
I've promised to get a library out there, and here it is. The base URL
is https://github.com/Shachar/safewrite, and the actual code is at
https://github.com/Shachar/safewrite/blob/master/safewrite.c
Some things I noticed:
That way one could use
[..]
typedef struct {
int fd;
char buffer[PATH_MAX];
} safe_t;
Except, you can't rely on PATH_MAX on any modern system. It's defined in
Linux headers to an arbitrary value to make old code compile, but for
example Hurd folks decided to not define it altogether to make buggy code
fail during compilation rather than on runtime.
Right, so
typedef struct {
int fd;
char buffer[0];
} safe_t;
and allocating the struct as big as needed.
Maybe don't recommend invalid C? Bad habits don't have to live on forever...
HS
Would you use
typedef struct {
int fd;
char buffer[];
} safe_t;
or what do you mean by invalid C?
char buffer[0]; is veeery gcc-specific as the storage size of buffer
is 0. According to the C99 standard:
6.7.5.2 Array declarators
Constraints
1 In addition to optional type qualifiers and the keyword static, the [ and
] may delimit an expression or *. If they delimit an expression (which
specifies the size of an array), the expression shall have an integer type.
If the expression is a constant expression, it shall have a value greater
than zero.
Either make this char buffer[1]; and live with the fact that e.g.
cppcheck will yell at you (better not), or use safe_t x= ...; char
*buffer = x + 1; with or without explicit reference in safe_t (if you
want to allocate in one block) or simply allocate it seperately.
BTW: KDE4 is a very good example for failure with modern filesystems.
I regularly loose configuration files when suspend-to-ram fails even
if the configuration of the running programs were not changed. Yay :-(
And this is with XFS, not Ext4! Filed a bug a looong time ago in
KDE BTS. Reaction: none!
HS
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110126173619.185277zxg7ixv...@v1539.ncsrv.de
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 5:36 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! Maybe complain to the Linux kernel people instead. -- Olaf -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/AANLkTik20C6mCB=vc-pych37zyr_9wexhd5mg5vbw...@mail.gmail.com
Re: Safe file update library ready (sort of)
Zitat von Olaf van der Spek olafvds...@gmail.com: On Wed, Jan 26, 2011 at 5:36 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! Maybe complain to the Linux kernel people instead. suspend-to-ram fails is the same situation as a sudden power loss. In this case, it's not a kernel bug. I don't know how KDE4 handles its configuration files but something is wrong there when I loose config data without changing any settings in this scenario... HS -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110126192542.91195o63nouch...@v1539.ncsrv.de
Re: Safe file update library ready (sort of)
On Wed, Jan 26, 2011 at 7:25 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: Zitat von Olaf van der Spek olafvds...@gmail.com: On Wed, Jan 26, 2011 at 5:36 PM, Hendrik Sattler p...@hendrik-sattler.de wrote: BTW: KDE4 is a very good example for failure with modern filesystems. I regularly loose configuration files when suspend-to-ram fails even if the configuration of the running programs were not changed. Yay :-( And this is with XFS, not Ext4! Filed a bug a looong time ago in KDE BTS. Reaction: none! Maybe complain to the Linux kernel people instead. suspend-to-ram fails is the same situation as a sudden power loss. In this case, it's not a kernel bug. I don't know how KDE4 handles its configuration files but something is wrong there when I loose config data without changing any settings in this scenario... I know. It's about a(nother) use case for O_ATOMIC, but the kernel guys prefer workarounds. -- Olaf -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/AANLkTi=+2Dffvgg5=bucrwfy7qrrjuwso-q7awf0w...@mail.gmail.com
Re: Safe file update library ready (sort of)
On 04/01/11 16:24, Ian Jackson wrote: Shachar Shemesh writes (Safe file update library ready (sort of)): This is not a formal release just yet (plus one function is still missing an implementation, trivial though it might be). It's just that the list obviously has a few people knowledgeable on the subject who can give my code a second look and see whether there is anything I have missed. I think this kind of approach is the wrong way to solve most instances of this kind of problem. A better way is userv, which I wrote over a decade ago and have been using with success on chiark since. Ian. I'm sorry, it might be me, but I fail to see the overlap between the functionalities of safewrite vs. userv. The premises for safewrite is that a program wants to make sure data integrity is maintained when writing files. Userv seems to be about trust and a user level tool. The two seem to revolve around two completely different interpretations to the word safe, as well as two completely different use scenarios. Am I missing something here? Shachar -- Shachar Shemesh Lingnu Open Source Consulting Ltd. http://www.lingnu.com -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4d2330be.2070...@debian.org
Re: Safe file update library ready (sort of)
Shachar Shemesh writes (Safe file update library ready (sort of)): This is not a formal release just yet (plus one function is still missing an implementation, trivial though it might be). It's just that the list obviously has a few people knowledgeable on the subject who can give my code a second look and see whether there is anything I have missed. I think this kind of approach is the wrong way to solve most instances of this kind of problem. A better way is userv, which I wrote over a decade ago and have been using with success on chiark since. Ian. -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/19747.11688.539411.691...@chiark.greenend.org.uk
Re: Safe file update library ready (sort of)
Shachar Shemesh writes (Re: Safe file update library ready (sort of)): I'm sorry, it might be me, but I fail to see the overlap between the functionalities of safewrite vs. userv. The premises for safewrite is that a program wants to make sure data integrity is maintained when writing files. Userv seems to be about trust and a user level tool. The two seem to revolve around two completely different interpretations to the word safe, as well as two completely different use scenarios. Am I missing something here? Sorry, I replied to the wrong message. I meant to reply to Adam Borowski's comment, where he wrote: ] There's a race condition: ] ] while [ 1 ]; do ln -s /etc/passwd somefile.tmp; done ] Hey root, could you please use this program using libsafewrite on ] 'somefile'? Having said that, I don't think the concept behind your library is sound, because it presupposes that all previous programs which update files are buggy. Just because some wrongheaded Linux kernel filesystem developers think that almost all previously written Unix programs are buggy, doesn't mean that it's true or that the right fix is to rewrite every program. Ian. -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/19747.25756.63354.32...@chiark.greenend.org.uk
Re: Safe file update library ready (sort of)
On Tue, Jan 4, 2011 at 7:19 PM, Ian Jackson ijack...@chiark.greenend.org.uk wrote: Having said that, I don't think the concept behind your library is sound, because it presupposes that all previous programs which update files are buggy. They are. Kinda. They either do unsafe file updates or they have regressions from the unsafe file update case. Just because some wrongheaded Linux kernel filesystem developers think that almost all previously written Unix programs are buggy, doesn't mean that it's true or that the right fix is to rewrite every program. This lib is about making the temp file workaround easier. Olaf -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/aanlkti=1hflnzt3gdpqacp5jq_nv28iur-5bfu0ha...@mail.gmail.com
Safe file update library ready (sort of)
Hi all, I've promised to get a library out there, and here it is. The base URL is https://github.com/Shachar/safewrite, and the actual code is at https://github.com/Shachar/safewrite/blob/master/safewrite.c This is not a formal release just yet (plus one function is still missing an implementation, trivial though it might be). It's just that the list obviously has a few people knowledgeable on the subject who can give my code a second look and see whether there is anything I have missed. I'll probably make an official release over the next couple of days. Feedback most appreciated. Shachar -- Shachar Shemesh Lingnu Open Source Consulting Ltd. http://www.lingnu.com -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4d21a84c.4060...@debian.org
Re: Safe file update library ready (sort of)
On Mon, Jan 03, 2011 at 12:43:24PM +0200, Shachar Shemesh wrote: Hi all, I've promised to get a library out there, and here it is. The base URL is https://github.com/Shachar/safewrite, and the actual code is at https://github.com/Shachar/safewrite/blob/master/safewrite.c [...] Give my code a second look and see whether there is anything I have missed. There's a race condition: while [ 1 ]; do ln -s /etc/passwd somefile.tmp; done Hey root, could you please use this program using libsafewrite on 'somefile'? -- 1KB // Microsoft corollary to Hanlon's razor: // Never attribute to stupidity what can be // adequately explained by malice. signature.asc Description: Digital signature
Re: Safe file update library ready (sort of)
On 03/01/11 14:10, Adam Borowski wrote: There's a race condition: while [ 1 ]; do ln -s /etc/passwd somefile.tmp; done Hey root, could you please use this program using libsafewrite on 'somefile'? Two questions: 1. Is this race a regression from the single file case? 2. Is this race avoidable? In essence, it is impossible, as far as I know (patches welcome) to avoid a race when symlinks are involved (with specific exceptions). The assumption is, and has always been, that the directory resides inside a location that is secure from attacks. In this particular case, for example, you don't need this race at all. Simply do ln -s /etc/passwd somefile and ask root to write to somefile, with or without safewrite. That would work equally well, and does not require to race with anything. You might be wondering, if that is the case, why I'm unlinking somefile.tmp before opening it with O_CREAT|O_TRUNC. The reason is that it might have permissions (say, from a previous run that failed - unlikely, but not impossible) that prevent proper functioning. It has nothing to do with permissions. Shachar -- Shachar Shemesh Lingnu Open Source Consulting Ltd. http://www.lingnu.com -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4d21d59c.3010...@debian.org
Re: Safe file update library ready (sort of)
On 03/01/11 14:10, Adam Borowski wrote: There's a race condition: while [ 1 ]; do ln -s /etc/passwd somefile.tmp; done Hey root, could you please use this program using libsafewrite on 'somefile'? Two questions: 1. Is this race a regression from the single file case? 2. Is this race avoidable? In essence, it is impossible, as far as I know (patches welcome) to avoid a race when symlinks are involved (with specific exceptions). The assumption is, and has always been, that the directory resides inside a location that is secure from attacks. In this particular case, for example, you don't need this race at all. Simply do ln -s /etc/passwd somefile and ask root to write to somefile, with or without safewrite. That would work equally well, and does not require to race with anything. You might be wondering, if that is the case, why I'm unlinking somefile.tmp before opening it with O_CREAT|O_TRUNC. The reason is that it might have permissions (say, from a previous run that failed - unlikely, but not impossible) that prevent proper functioning. It has nothing to do with permissions. Shachar -- Shachar Shemesh Lingnu Open Source Consulting Ltd. http://www.lingnu.com -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4d21d57d.1030...@shemesh.biz
Re: Safe file update library ready (sort of)
Hi Dne Mon, 03 Jan 2011 15:56:44 +0200 Shachar Shemesh shac...@debian.org napsal(a): In essence, it is impossible, as far as I know (patches welcome) to avoid a race when symlinks are involved (with specific exceptions). The assumption is, and has always been, that the directory resides inside a location that is secure from attacks. In this particular case, for example, you don't need this race at all. Simply do ln -s /etc/passwd somefile and ask root to write to somefile, with or without safewrite. That would work equally well, and does not require to race with anything. You might be wondering, if that is the case, why I'm unlinking somefile.tmp before opening it with O_CREAT|O_TRUNC. The reason is that it might have permissions (say, from a previous run that failed - unlikely, but not impossible) that prevent proper functioning. It has nothing to do with permissions. I think what you are missing is (at least) O_NOFOLLOW. -- Michal Čihař | http://cihar.com | http://blog.cihar.com signature.asc Description: PGP signature
