[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Safe file update library ready (sort of)

Zitat von "Goswin von Brederlow" <goswin-v-b@web.de>:


"Hendrik Sattler" <post@hendrik-sattler.de> writes:


Zitat von "Goswin von Brederlow" <goswin-v-b@web.de>:


Adam Borowski <kilobyte@angband.pl> writes:


On Wed, Jan 26, 2011 at 12:03:52PM +0100, Goswin von Brederlow wrote:

Shachar Shemesh <shachar@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 looooooong time ago in KDE BTS. Reaction: none! 

HS
Reply to: