Re: [PATCH v6 2/7] VFS: Add O_DENYDELETE support for VFS
On Fri, 26 Apr 2013 16:04:16 +0400 Pavel Shilovsky wrote: > Introduce new LOCK_DELETE flock flag that is suggested to be used > internally only to map O_DENYDELETE open flag: > > !O_DENYDELETE -> LOCK_DELETE | LOCK_MAND. > > Signed-off-by: Pavel Shilovsky > --- > fs/locks.c | 53 > +--- > fs/namei.c | 3 +++ > include/linux/fs.h | 6 + > include/uapi/asm-generic/fcntl.h | 1 + > 4 files changed, 54 insertions(+), 9 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index dbc5557..1cc68a9 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -269,7 +269,7 @@ EXPORT_SYMBOL(locks_copy_lock); > > static inline int flock_translate_cmd(int cmd) { > if (cmd & LOCK_MAND) > - return cmd & (LOCK_MAND | LOCK_RW); > + return cmd & (LOCK_MAND | LOCK_RW | LOCK_DELETE); > switch (cmd) { > case LOCK_SH: > return F_RDLCK; > @@ -614,6 +614,8 @@ deny_flags_to_cmd(unsigned int flags) > cmd |= LOCK_READ; > if (!(flags & O_DENYWRITE)) > cmd |= LOCK_WRITE; > + if (!(flags & O_DENYDELETE)) > + cmd |= LOCK_DELETE; > > return cmd; > } > @@ -836,6 +838,31 @@ out: > return error; > } > > +int > +sharelock_may_delete(struct dentry *dentry) > +{ > + struct file_lock **before; > + int rc = 0; > + > + if (!IS_SHARELOCK(dentry->d_inode)) > + return rc; > + > + lock_flocks(); > + for_each_lock(dentry->d_inode, before) { > + struct file_lock *fl = *before; > + if (IS_POSIX(fl)) > + break; > + if (IS_LEASE(fl)) > + continue; > + if (fl->fl_type & LOCK_DELETE) > + continue; Are you sure this logic is right? What if I have a normal non-LOCK_MAND lock on this file. Won't that prevent me from deleting it with this patch? > + rc = 1; > + break; > + } > + unlock_flocks(); > + return rc; > +} > + > /* > * Determine if a file is allowed to be opened with specified access and > share > * modes. Lock the file and return 0 if checks passed, otherwise return > @@ -850,10 +877,6 @@ sharelock_lock_file(struct file *filp) > if (!IS_SHARELOCK(filp->f_path.dentry->d_inode)) > return error; > > - /* Disable O_DENYDELETE support for now */ > - if (filp->f_flags & O_DENYDELETE) > - return -EINVAL; > - > error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags)); > if (error) > return error; > @@ -1717,6 +1740,12 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, > cmd) > if (!f.file) > goto out; > > + /* LOCK_DELETE is defined to be translated from O_DENYDELETE only */ > + if (cmd & LOCK_DELETE) { > + error = -EINVAL; > + goto out; > + } > + > can_sleep = !(cmd & LOCK_NB); > cmd &= ~LOCK_NB; > unlock = (cmd == LOCK_UN); > @@ -2261,10 +2290,16 @@ static void lock_get_status(struct seq_file *f, > struct file_lock *fl, > seq_printf(f, "UNKNOWN UNKNOWN "); > } > if (fl->fl_type & LOCK_MAND) { > - seq_printf(f, "%s ", > -(fl->fl_type & LOCK_READ) > -? (fl->fl_type & LOCK_WRITE) ? "RW " : "READ " > -: (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE > "); > + if (fl->fl_type & LOCK_DELETE) > + seq_printf(f, "%s ", > + (fl->fl_type & LOCK_READ) ? > + (fl->fl_type & LOCK_WRITE) ? "RWDEL" : "RDDEL" : > + (fl->fl_type & LOCK_WRITE) ? "WRDEL" : "DEL "); > + else > + seq_printf(f, "%s ", > + (fl->fl_type & LOCK_READ) ? > + (fl->fl_type & LOCK_WRITE) ? "RW " : "READ " : > + (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE "); > } else { > seq_printf(f, "%s ", > (lease_breaking(fl)) > diff --git a/fs/namei.c b/fs/namei.c > index dd236fe..a404f7d 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -2220,6 +2220,7 @@ static inline int check_sticky(struct inode *dir, > struct inode *inode) > * 9. We can't remove a root or mountpoint. > * 10. We don't allow removal of NFS sillyrenamed files; it's handled by > * nfs_async_unlink(). > + * 11. We can't do it if victim is locked by O_DENYDELETE sharelock. > */ > static int may_delete(struct inode *dir,struct dentry *victim,int isdir) > { > @@ -2250,6 +2251,8 @@ static int may_delete(struct inode *dir,struct dentry > *victim,int isdir) > return -ENOENT; > if (victim->d_flags & DCACHE_NFSFS_RENAMED) > return -EBUSY; >
Re: [PATCH v6 1/7] VFS: Introduce new O_DENY* open flags
On Fri, 26 Apr 2013 16:04:15 +0400 Pavel Shilovsky wrote: > This patch adds 3 flags: > 1) O_DENYREAD that doesn't permit read access, > 2) O_DENYWRITE that doesn't permit write access, > 3) O_DENYDELETE that doesn't permit delete or rename. > > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - > this change can benefit cifs and nfs modules as well as Samba and > NFS file servers that export the same directory for Windows clients, > or Wine applications that access the same files simultaneously. > > These flags are only take affect for opens on mounts with new sharelock > option. They are translated to flock's flags: > > !O_DENYREAD -> LOCK_READ | LOCK_MAND > !O_DENYWRITE -> LOCK_WRITE | LOCK_MAND > > and set through flock_lock_file on a file. If the file can't be locked > due conflicts with another open with O_DENY* flags, a new -ESHAREDENIED > error code is returned. > > Create codepath is slightly changed to prevent data races on > newely created files: when open with O_CREAT can return -ESHAREDENIED > error for successfully created files due to a sharelock set by > another task. > > Temporary disable O_DENYDELETE support - will enable it in further patches. > > Signed-off-by: Pavel Shilovsky > --- > fs/fcntl.c | 5 ++- > fs/locks.c | 97 > +--- > fs/namei.c | 45 +-- > fs/proc_namespace.c | 1 + > include/linux/fs.h | 7 +++ > include/uapi/asm-generic/errno.h | 2 + > include/uapi/asm-generic/fcntl.h | 11 + > include/uapi/linux/fs.h | 1 + > 8 files changed, 156 insertions(+), 13 deletions(-) > > diff --git a/fs/fcntl.c b/fs/fcntl.c > index 6599222..a1eee47 100644 > --- a/fs/fcntl.c > +++ b/fs/fcntl.c > @@ -730,14 +730,15 @@ static int __init fcntl_init(void) >* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY >* is defined as O_NONBLOCK on some platforms and not on others. >*/ > - BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32( > + BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32( > O_RDONLY| O_WRONLY | O_RDWR| > O_CREAT | O_EXCL| O_NOCTTY | > O_TRUNC | O_APPEND | /* O_NONBLOCK | */ > __O_SYNC| O_DSYNC | FASYNC| > O_DIRECT| O_LARGEFILE | O_DIRECTORY | > O_NOFOLLOW | O_NOATIME | O_CLOEXEC | > - __FMODE_EXEC| O_PATH > + __FMODE_EXEC| O_PATH| O_DENYREAD| > + O_DENYWRITE | O_DENYDELETE > )); > > fasync_cache = kmem_cache_create("fasync_cache", > diff --git a/fs/locks.c b/fs/locks.c > index cb424a4..dbc5557 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -605,20 +605,73 @@ static int posix_locks_conflict(struct file_lock > *caller_fl, struct file_lock *s > return (locks_conflict(caller_fl, sys_fl)); > } > > -/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific > - * checking before calling the locks_conflict(). > +static unsigned int > +deny_flags_to_cmd(unsigned int flags) > +{ > + unsigned int cmd = LOCK_MAND; > + > + if (!(flags & O_DENYREAD)) > + cmd |= LOCK_READ; > + if (!(flags & O_DENYWRITE)) > + cmd |= LOCK_WRITE; > + > + return cmd; > +} > + > +/* > + * locks_mand_conflict - Determine if there's a share reservation conflict > + * @caller_fl: lock we're attempting to acquire > + * @sys_fl: lock already present on system that we're checking against > + * > + * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE > + * tell us whether the reservation allows other readers and writers. > + */ > +static int > +locks_mand_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) > +{ > + unsigned char caller_type = caller_fl->fl_type; > + unsigned char sys_type = sys_fl->fl_type; > + fmode_t caller_fmode = caller_fl->fl_file->f_mode; > + fmode_t sys_fmode = sys_fl->fl_file->f_mode; > + > + /* they can only conflict if FS is mounted with MS_SHARELOCK */ > + if (!IS_SHARELOCK(caller_fl->fl_file->f_path.dentry->d_inode)) > + return 0; > + > + /* they can only conflict if they're both LOCK_MAND */ > + if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND)) > + return 0; > + > + if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ)) > + return 1; > + if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE)) > + return 1; > + if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ)) > + return 1; > + if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE)) > + return 1; > + > + return 0; > +} > + > +/* > + * Determine if lock sys_fl blocks lock caller_fl.
Possible wine bug concerning the case of the DbgHelp.h header filename
I successfully built the ultra-fast ninja build tool on Wine using the MinGW g++ compiler. To achieve that success I had to deal with a small number of issues including one wine/ninja header name inconsistency which is that DbgHelp.h (#included by the ninja code) has a lower-case name (dbghelp.h) on wine, and the MinGW suite of compilers is sensitive to the case of header file names. I worked around this issue with the following symlink /home/wine/wine_build/install-git/include/wine/windows/DbgHelp.h -> dbghelp.h where /home/wine/wine_build/install-git is my install prefix for my wine-git build. Is Wine following the correct Windows naming convention for this header? For what it is worth, I did a google search for and most of the hits were for DbgHelp.h rather than dbghelp.h so I can understand why the ninja developers used #include rather than the lower-case version of that name. If the wine developers here decide this is definitely a wine issue, I am willing to write up the bug report on your bugtracker so this issue doesn't get lost. A search there for did not turn up anything relevant. Alan __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __ Linux-powered Science __
Re: [PATCH] d3dx9_36: Implement D3DXGetShaderInputSemantics + tests.
2013/6/11 Christian Costa : > Fixes bug 22682. > --- > dlls/d3dx9_36/d3dx9_36.spec |2 +- > dlls/d3dx9_36/shader.c | 50 ++ > dlls/d3dx9_36/tests/shader.c | 55 > ++ > include/d3dx9shader.h|1 + > 4 files changed, 107 insertions(+), 1 deletion(-) > > diff --git a/dlls/d3dx9_36/d3dx9_36.spec b/dlls/d3dx9_36/d3dx9_36.spec > index f2c229a..0f1387e 100644 > --- a/dlls/d3dx9_36/d3dx9_36.spec > +++ b/dlls/d3dx9_36/d3dx9_36.spec > @@ -159,7 +159,7 @@ > @ stdcall D3DXGetPixelShaderProfile(ptr) > @ stdcall D3DXGetShaderConstantTable(ptr ptr) > @ stdcall D3DXGetShaderConstantTableEx(ptr long ptr) > -@ stub D3DXGetShaderInputSemantics(ptr ptr ptr) > +@ stdcall D3DXGetShaderInputSemantics(ptr ptr ptr) > @ stub D3DXGetShaderOutputSemantics(ptr ptr ptr) > @ stdcall D3DXGetShaderSamplers(ptr ptr ptr) > @ stdcall D3DXGetShaderSize(ptr) > diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c > index 75bc9b5..3b71b4b 100644 > --- a/dlls/d3dx9_36/shader.c > +++ b/dlls/d3dx9_36/shader.c > @@ -1,6 +1,7 @@ > /* > * Copyright 2008 Luis Busquets > * Copyright 2009 Matteo Bruni > + * Copyright 2010, 2013 Christian Costa > * Copyright 2011 Travis Athougies > * > * This library is free software; you can redistribute it and/or > @@ -1778,3 +1779,52 @@ HRESULT WINAPI D3DXGetShaderSamplers(const DWORD > *byte_code, const char **sample > > return D3D_OK; > } > + > +HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code, > D3DXSEMANTIC *semantics, UINT *count) > +{ > +const DWORD *ptr = byte_code; > +UINT i = 0; > + > +TRACE("byte_code = %p, semantics = %p, count = %p\n", byte_code, > semantics, count); > + > +if (!byte_code) > +return D3DERR_INVALIDCALL; > + > +TRACE("Shader version: %#x\n", *ptr); > + > +/* Look for the END token, skipping the VERSION token */ > +while (*++ptr != D3DSIO_END) I think you should be a bit more careful in skipping non-opcode DWORDs, otherwise you could e.g. potentially match with constants from DEF instructions - very unlikely to happen in practice, sure, but since it can be easily avoided, why not? Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can probably get away without having to write a table with the parameters count for each SM1 opcode by just skipping DWORDs with the most significant bit set (see shader_skip_unrecognized() from the same file). Of course you still have to special case DEF but that should be it. > +{ > +/* Skip comments */ > +if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_COMMENT) > +{ > +ptr += ((*ptr & D3DSI_COMMENTSIZE_MASK) >> > D3DSI_COMMENTSIZE_SHIFT); > +} > +else if ((*ptr & D3DSI_OPCODE_MASK) == D3DSIO_DCL) > +{ > +DWORD param1 = *++ptr; > +DWORD param2 = *++ptr; > +DWORD usage = param1 & 0x1f; > +DWORD usage_index = (param1 >> 16) & 0xf; > +DWORD reg_type = (((param2 >> 11) & 0x3) << 3) | ((param2 >> 28) > & 0x7); > + > +TRACE("D3DSIO_DCL param1: %#x, param2: %#x, usage: %u, > usage_index: %u, reg_type: %u\n", > + param1, param2, usage, usage_index, reg_type); > + > +if (reg_type == D3DSPR_INPUT) > +{ > +if (semantics) > +{ > +semantics[i].Usage = usage; > +semantics[i].UsageIndex = usage_index; > +} > +i++; > +} > +} > +} > + > +if (count) > +*count = i; > + > +return D3D_OK; > +} Have you tried to implement D3DXGetShaderOutputSemantics too? I suspect most of the code will be pretty much the same, in that case you could move the common code to a helper function and use it from both. You don't necessarily need to do this right now, just mentioning a potential follow-up. > diff --git a/dlls/d3dx9_36/tests/shader.c b/dlls/d3dx9_36/tests/shader.c > index f7be174..471cd8c 100644 > --- a/dlls/d3dx9_36/tests/shader.c > +++ b/dlls/d3dx9_36/tests/shader.c > @@ -1,5 +1,6 @@ > /* > * Copyright 2008 Luis Busquets > + * Copyright 2010, 2013 Christian Costa > * Copyright 2011 Travis Athougies > * > * This library is free software; you can redistribute it and/or > @@ -271,6 +272,22 @@ static const D3DXCONSTANT_DESC ctab_samplers_expected[] > = { > {"sampler2", D3DXRS_SAMPLER, 3, 1, D3DXPC_OBJECT, D3DXPT_SAMPLER3D, 1, > 1, 1, 0, 4, NULL}, > {"notsampler", D3DXRS_FLOAT4, 2, 1, D3DXPC_VECTOR, D3DXPT_FLOAT, 1, > 4, 1, 0, 16, NULL}}; > > +static const DWORD semantics_vs11[] = { > +0xfffe0101, > /* vs_1_1 */ > +0x001f, 0x8000, 0x900f, > /* dcl_position0 v0 (input) */ > +0x001f, 0x8005, 0x900f
Re: [PATCH] d3dx9_36: Set compilation_errors to NULL when no error encountered + tests.
On 11.06.2013 22:08, Christian Costa wrote: Fixes bug 26598. --- dlls/d3dx9_36/effect.c |4 dlls/d3dx9_36/tests/effect.c | 17 + 2 files changed, 21 insertions(+) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 1924c07..bab4560 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -5792,6 +5792,10 @@ HRESULT WINAPI D3DXCreateEffectEx(struct IDirect3DDevice9 *device, const void *s *effect = &object->ID3DXEffect_iface; +/* Must be set to NULL if no compilation error */ +if (compilation_errors) +*compilation_errors = NULL; + No, this is wrong! Your test case doesn't cover all cases. Cheers Rico
Re: [PATCH] d3dx9_36: Implement D3DXGetShaderInputSemantics + tests.
On 11.06.2013 22:08, Christian Costa wrote: +TRACE("byte_code = %p, semantics = %p, count = %p\n", byte_code, semantics, count); The rest of the file seems to use the trace without the "=". +ok(ret == D3D_OK, "Failed with %#xn", ret); +ok(count == 3, "Got %u, expected 1\n", count); Should be "\n" and "expected 3". Both several times. As the return value for semantics is in both tests the same, it might be useful to initialize the semantics first to check that the values are really set in both cases. What does D3DXGetShaderInputSemantics(semantics_vs11, NULL, NULL); and D3DXGetShaderInputSemantics(NULL, NULL, NULL); return? Well both are corner cases without much meaning. You may check for the semantics where the count is NULL like D3DXGetShaderInputSemantics(semantics_vs11, semantics, NULL); Cheers Rico
Re: [PATCH v6 2/7] VFS: Add O_DENYDELETE support for VFS
2013/6/11 Jeff Layton : > On Fri, 26 Apr 2013 16:04:16 +0400 > Pavel Shilovsky wrote: > >> Introduce new LOCK_DELETE flock flag that is suggested to be used >> internally only to map O_DENYDELETE open flag: >> >> !O_DENYDELETE -> LOCK_DELETE | LOCK_MAND. >> >> Signed-off-by: Pavel Shilovsky >> --- >> fs/locks.c | 53 >> +--- >> fs/namei.c | 3 +++ >> include/linux/fs.h | 6 + >> include/uapi/asm-generic/fcntl.h | 1 + >> 4 files changed, 54 insertions(+), 9 deletions(-) >> >> diff --git a/fs/locks.c b/fs/locks.c >> index dbc5557..1cc68a9 100644 >> --- a/fs/locks.c >> +++ b/fs/locks.c >> @@ -269,7 +269,7 @@ EXPORT_SYMBOL(locks_copy_lock); >> >> static inline int flock_translate_cmd(int cmd) { >> if (cmd & LOCK_MAND) >> - return cmd & (LOCK_MAND | LOCK_RW); >> + return cmd & (LOCK_MAND | LOCK_RW | LOCK_DELETE); >> switch (cmd) { >> case LOCK_SH: >> return F_RDLCK; >> @@ -614,6 +614,8 @@ deny_flags_to_cmd(unsigned int flags) >> cmd |= LOCK_READ; >> if (!(flags & O_DENYWRITE)) >> cmd |= LOCK_WRITE; >> + if (!(flags & O_DENYDELETE)) >> + cmd |= LOCK_DELETE; >> >> return cmd; >> } >> @@ -836,6 +838,31 @@ out: >> return error; >> } >> >> +int >> +sharelock_may_delete(struct dentry *dentry) >> +{ >> + struct file_lock **before; >> + int rc = 0; >> + >> + if (!IS_SHARELOCK(dentry->d_inode)) >> + return rc; >> + >> + lock_flocks(); >> + for_each_lock(dentry->d_inode, before) { >> + struct file_lock *fl = *before; >> + if (IS_POSIX(fl)) >> + break; >> + if (IS_LEASE(fl)) >> + continue; >> + if (fl->fl_type & LOCK_DELETE) >> + continue; > > Are you sure this logic is right? What if I have a normal non-LOCK_MAND > lock on this file. Won't that prevent me from deleting it with this > patch? > It is a bug, thank you for pointing it out. We need to skip all non-LOCK_MAND locks here. -- Best regards, Pavel Shilovsky.
Patch 96712
Hi, Is there something wrong with this patch? The only feedback I got is a typo (fixed). http://source.winehq.org/patches/data/96712 Thanks
Re: [PATCH 1/1] wined3d: Introduce a helper function for printing floats in GLSL shaders.
On 11 June 2013 09:52, Stefan Dösinger wrote: > I think this should be added as a code comment above shader_glsl_ftoa, I > don't think the reason why we need our own conversion function is obvious. > I originally considered adding a comment, but figured it was obvious enough from being in the context of GLSL shader generation. I don't feel particularly strongly about it though, so I can add it again if it helps. > The ARB shader backend will need this as well, but I can take care of this. > What external software triggers this behavior? > I'm not sure, this was reported on IRC. But you can easily reproduce it by e.g. adding setlocale(LC_ALL, ""); to wined3d_dll_init() and running the tests with an appropriate locale set. > shader_glsl_ftoa doesn't handle inf or nan. I assume that's intentional > because they would only trigger a parser error. > Yeah, inf and nan aren't really meaningful here. If we really wanted to it would only be a couple of extra lines though.
Re: [PATCH 1/1] wined3d: Introduce a helper function for printing floats in GLSL shaders.
Am 11.06.2013 um 09:27 schrieb Henri Verbeet: > We always want to use '.' as decimal separator in GLSL, instead of the locale > specific one. Reported on IRC. I think this should be added as a code comment above shader_glsl_ftoa, I don't think the reason why we need our own conversion function is obvious. The ARB shader backend will need this as well, but I can take care of this. What external software triggers this behavior? > case WINED3D_DATA_FLOAT: > -sprintf(register_name, "%.8e", *(const float > *)reg->immconst_data); > +shader_glsl_ftoa(*(const float > *)reg->immconst_data, register_name); > break; shader_glsl_ftoa doesn't handle inf or nan. I assume that's intentional because they would only trigger a parser error.