Loading ...
Sorry, an error occurred while loading the content.
 

windows bug: filewritable() returns 0 if we use it on the current script being read

Expand Messages
  • Philippe Vaucher
    Hello, I think I ve found a bug for vim under windows. Basically if inside a vim script you do filewritable(expand( )) it returns 0 even if the file
    Message 1 of 16 , Nov 2, 2010
      Hello,

      I think I've found a bug for vim under windows.

      Basically if inside a vim script you do
      "filewritable(expand('<sfile>'))" it returns 0 even if the file is
      writable.

      I read the source and basically it boils down to that the dwShareMode
      param of CreateFile() is 0, when it should be FILE_SHARE_READ|
      FILE_SHARE_WRITE:

      hFile = CreateFileW(wn, am, 0, NULL, OPEN_EXISTING, 0, NULL);

      I'm not sure of the implications, but basically I think just changing
      the dwShareMode param to the correct value everywhere CreateFile() is
      used should fix it. Not that in certain places it's used with the
      correct flag value.

      Philippe

      --
      You received this message from the "vim_dev" maillist.
      Do not top-post! Type your reply below the text you are replying to.
      For more information, visit http://www.vim.org/maillist.php
    • Philippe Vaucher
      Hum? Should I submit a patch? This is a bug right? -- You received this message from the vim_dev maillist. Do not top-post! Type your reply below the text
      Message 2 of 16 , Nov 3, 2010
        Hum? Should I submit a patch? This is a bug right?

        --
        You received this message from the "vim_dev" maillist.
        Do not top-post! Type your reply below the text you are replying to.
        For more information, visit http://www.vim.org/maillist.php
      • Bram Moolenaar
        ... It makes sense, but we must make sure this does not cause any trouble. Does it work on all Windows versions? On Unix we never care about others having the
        Message 3 of 16 , Nov 3, 2010
          Philippe Vaucher wrote:

          > I think I've found a bug for vim under windows.
          >
          > Basically if inside a vim script you do
          > "filewritable(expand('<sfile>'))" it returns 0 even if the file is
          > writable.
          >
          > I read the source and basically it boils down to that the dwShareMode
          > param of CreateFile() is 0, when it should be FILE_SHARE_READ|
          > FILE_SHARE_WRITE:
          >
          > hFile = CreateFileW(wn, am, 0, NULL, OPEN_EXISTING, 0, NULL);
          >
          > I'm not sure of the implications, but basically I think just changing
          > the dwShareMode param to the correct value everywhere CreateFile() is
          > used should fix it. Not that in certain places it's used with the
          > correct flag value.

          It makes sense, but we must make sure this does not cause any trouble.
          Does it work on all Windows versions?

          On Unix we never care about others having the file open, thus I don't
          see a reason to check for that on MS-Windows.

          --
          Q: How do you tell the difference between a female cat and a male cat?
          A: You ask it a question and if HE answers, it's a male but, if SHE
          answers, it's a female.

          /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
          /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
          \\\ download, build and distribute -- http://www.A-A-P.org ///
          \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

          --
          You received this message from the "vim_dev" maillist.
          Do not top-post! Type your reply below the text you are replying to.
          For more information, visit http://www.vim.org/maillist.php
        • Ben Fritz
          On Nov 2, 7:21 am, Philippe Vaucher ... I m pretty sure you nomally WANT to lock a file from being written, while you are
          Message 4 of 16 , Nov 3, 2010
            On Nov 2, 7:21 am, Philippe Vaucher <philippe.vauc...@...>
            wrote:
            > Hello,
            >
            > I think I've found a bug for vim under windows.
            >
            > Basically if inside a vim script you do
            > "filewritable(expand('<sfile>'))" it returns 0 even if the file is
            > writable.
            >
            > I read the source and basically it boils down to that the dwShareMode
            > param of CreateFile() is 0, when it should be FILE_SHARE_READ|
            > FILE_SHARE_WRITE:
            >
            >   hFile = CreateFileW(wn, am, 0, NULL, OPEN_EXISTING, 0, NULL);
            >
            > I'm not sure of the implications, but basically I think just changing
            > the dwShareMode param to the correct value everywhere CreateFile() is
            > used should fix it. Not that in certain places it's used with the
            > correct flag value.
            >

            I'm pretty sure you nomally WANT to lock a file from being written,
            while you are executing its contents as a script. It sounds like this
            is what is happening. Is there some reason you want to write the file
            while executing it?

            --
            You received this message from the "vim_dev" maillist.
            Do not top-post! Type your reply below the text you are replying to.
            For more information, visit http://www.vim.org/maillist.php
          • Philippe Vaucher
            ... I don t know how I can make sure this doens t make any trouble... but if your question is if CreateFile() s dwShareMode flag is supported on all flavors of
            Message 5 of 16 , Nov 3, 2010
              > It makes sense, but we must make sure this does not cause any trouble.
              > Does it work on all Windows versions?

              I don't know how I can make sure this doens't make any trouble... but
              if your question is if CreateFile()'s dwShareMode flag is supported on
              all flavors of windows I think the answer is yes. Based on
              http://msdn.microsoft.com/en-us/library/aa363874(v=VS.85).aspx and on
              http://msdn.microsoft.com/en-us/library/aa363858(VS.85).aspx it looks
              like CreateFile goes back to windows 2000.

              I'm pretty new to digging into vim's source so I don't know if windows
              98 is supported (I was also a bit surprised by the old-style function
              declarations :).

              Anyway, I guess I could simply build vim and check if the problem goes
              away if I add the relevant flag everywhere CreateFile is used, but how
              do I make sure it didn't break other stuffs? My instincts tell me this
              should not create any problem as you'd normally not care about any
              file being already opened or not, but maybe I'm wrong, and I'm not
              sure about what I should test to assert at least the basic stuffs work
              as expected beside opening some files and checking my vim still
              works :)


              > On Unix we never care about others having the file open, thus I don't
              > see a reason to check for that on MS-Windows.

              Well yes on Unix the behavior is to permit others to open the file, so
              the current windows behavior differs in that respect.

              Thanks,
              Philippe

              --
              You received this message from the "vim_dev" maillist.
              Do not top-post! Type your reply below the text you are replying to.
              For more information, visit http://www.vim.org/maillist.php
            • Philippe Vaucher
              ... This bug arised from a script that checked wether itself was writable in order to decide to go and write to the parent directory or go to alternate routes
              Message 6 of 16 , Nov 3, 2010
                > I'm pretty sure you nomally WANT to lock a file from being written,
                > while you are executing its contents as a script. It sounds like this
                > is what is happening. Is there some reason you want to write the file
                > while executing it?

                This bug arised from a script that checked wether itself was writable
                in order to decide to go and write to the parent directory or go to
                alternate routes like ~/.vim etc. If you want more details I'll be
                happy to provide them but the point is that if such behavior is
                enabled in *nix it'd also be enabled in windows or disabled in both.

                The thing I'm really after is consistency, the fix for the particular
                script was to change it to check the script's directory instead which
                works consistently on both platforms.

                Philippe

                --
                You received this message from the "vim_dev" maillist.
                Do not top-post! Type your reply below the text you are replying to.
                For more information, visit http://www.vim.org/maillist.php
              • Craig Barkhouse
                ... Changing the share mode to (FILE_SHARE_READ | FILE_SHARE_WRITE) in mch_access() is theoretically safe, because mch_access() doesn t do any I/O and closes
                Message 7 of 16 , Nov 3, 2010
                  Phillippe Vaucher wrote:

                  > > I'm pretty sure you nomally WANT to lock a file from being written,
                  > > while you are executing its contents as a script. It sounds like this
                  > > is what is happening. Is there some reason you want to write the file
                  > > while executing it?
                  >
                  > This bug arised from a script that checked wether itself was writable in order to
                  > decide to go and write to the parent directory or go to alternate routes like
                  > ~/.vim etc. If you want more details I'll be happy to provide them but the point is
                  > that if such behavior is enabled in *nix it'd also be enabled in windows or
                  > disabled in both.
                  >
                  > The thing I'm really after is consistency, the fix for the particular script was to
                  > change it to check the script's directory instead which works consistently on
                  > both platforms.
                  >
                  > Philippe

                  Changing the share mode to (FILE_SHARE_READ | FILE_SHARE_WRITE) in mch_access() is theoretically safe, because mch_access() doesn't do any I/O and closes the handle immediately. It would not be ok to make a broader change throughout the source code to always open handles with more permissive sharing. When you use those share modes, basically you're saying "I'm opening a file handle now, and I really don't care if anyone else is reading and/or writing the file at the same time as I'm working with the file." In general you do care. If you're writing to a file, you don't want another process also writing, which would result in undefined file contents. You don't really want one process reading while another process is writing either, because then the reader will see an inconsistent view. The only type of sharing that is typically safe is read-read.

                  Assuming you don't want permissive sharing when doing actual I/O (I argue above that you don't), I question the value of changing mch_access() in the proposed way. The point of mch_access() is to give you a predictor of what types of access will likely work. If the access check tells you that opening the file for write will work, but then when you actually open it for write (using realistic sharing values) it fails, isn't this worse than what we have now?

                  The proposed change to mch_access() would only address a narrow scenario anyway. It allows you to avoid a sharing violation imposed by *your* handle. You'll still see sharing violations imposed by *other* handles. The general rule is that the access mode of any opened handle must be compatible with the share mode of all other opened handles.

                  Craig

                  --
                  You received this message from the "vim_dev" maillist.
                  Do not top-post! Type your reply below the text you are replying to.
                  For more information, visit http://www.vim.org/maillist.php
                • John Beckett
                  ... Although I haven t followed exactly what the OP is proposing, what Craig says is correct. Allowing multiple writers to the same file is only desirable
                  Message 8 of 16 , Nov 3, 2010
                    Craig Barkhouse wrote:
                    > Assuming you don't want permissive sharing when doing actual
                    > I/O (I argue above that you don't), I question the value of
                    > changing mch_access() in the proposed way. The point of
                    > mch_access() is to give you a predictor of what types of
                    > access will likely work. If the access check tells you that
                    > opening the file for write will work, but then when you
                    > actually open it for write (using realistic sharing values)
                    > it fails, isn't this worse than what we have now?

                    Although I haven't followed exactly what the OP is proposing,
                    what Craig says is correct. Allowing multiple writers to the
                    same file is only desirable under very planned circumstances
                    which do NOT include a text editor writing to a file.

                    John

                    --
                    You received this message from the "vim_dev" maillist.
                    Do not top-post! Type your reply below the text you are replying to.
                    For more information, visit http://www.vim.org/maillist.php
                  • Philippe Vaucher
                    ... As I said I m pretty fine with this way of seeing things, but then it means the bug is in the current *nix version. IMHO vim should behave as
                    Message 9 of 16 , Nov 4, 2010
                      > > Assuming you don't want permissive sharing when doing actual
                      > > I/O (I argue above that you don't), I question the value of
                      > > changing mch_access() in the proposed way.  The point of
                      > > mch_access() is to give you a predictor of what types of
                      > > access will likely work.  If the access check tells you that
                      > > opening the file for write will work, but then when you
                      > > actually open it for write (using realistic sharing values)
                      > > it fails, isn't this worse than what we have now?
                      >
                      > Although I haven't followed exactly what the OP is proposing,
                      > what Craig says is correct. Allowing multiple writers to the
                      > same file is only desirable under very planned circumstances
                      > which do NOT include a text editor writing to a file.

                      As I said I'm pretty fine with this way of seeing things, but then it
                      means the "bug" is in the current *nix version. IMHO vim should behave
                      as consistently as possible on the platforms it runs on, thus we'd
                      then change the *nix api to behave like the current win32 one.

                      Also, the way vim behaves on win32 about this is probably
                      inconsistent, as a test script I did the following:

                      echo 'writable: ' . filewritable(expand('<sfile>'))
                      write!

                      When I do ":source %" it output writable: 0, then writes the file
                      anyway :)

                      So I *think* that originally the vim authors never intended to prevent
                      other processes/whatever from begin able to open the same files, only
                      that on windows on some occasions the authors got lazy and forgot
                      about the dwSharedMode flag and just set it to 0. Someone should
                      decide wether we want to prevent it or not and then implement it on
                      all the platforms.

                      Philippe

                      --
                      You received this message from the "vim_dev" maillist.
                      Do not top-post! Type your reply below the text you are replying to.
                      For more information, visit http://www.vim.org/maillist.php
                    • Philippe Vaucher
                      So, who has to authority on this? Should we allow multiple writers to a file? Should we allow writers on currently read file? If no how drastics are the
                      Message 10 of 16 , Nov 7, 2010
                        So, who has to authority on this? Should we allow multiple writers to
                        a file? Should we allow writers on currently read file? If no how
                        drastics are the changes to be made to the linux api?

                        Philippe

                        --
                        You received this message from the "vim_dev" maillist.
                        Do not top-post! Type your reply below the text you are replying to.
                        For more information, visit http://www.vim.org/maillist.php
                      • Roland Puntaier
                        Hi Bram, After installing vim7.3 on a ubuntu system, I had again the problem that import did not work for .so libraries in lib-dynload. I found that sys.path
                        Message 11 of 16 , Nov 7, 2010
                          Hi Bram,

                          After installing vim7.3 on a ubuntu system, I had again the problem that
                          import did not work for .so libraries in lib-dynload. I found that
                          sys.path was initialized with "/usr/..." instead of "/usr/local/...". On
                          ubuntu (and probably on other linux distros as well) python3 is
                          installed in /usr/local while python2 is installed in /usr.

                          The attached patch calls Py_SetPythonHome with PYTHON3_PREFIX defined by
                          configure.
                          This solves the problem.

                          regards, Roland
                        • James Vega
                          ... That s not the case at all[0]. No distribution package should install to /usr/local as that s a reserved directory structure for the system
                          Message 12 of 16 , Nov 7, 2010
                            On Sun, Nov 07, 2010 at 06:04:35PM +0200, Roland Puntaier wrote:
                            > Hi Bram,
                            >
                            > After installing vim7.3 on a ubuntu system, I had again the problem
                            > that import did not work for .so libraries in lib-dynload. I found
                            > that sys.path was initialized with "/usr/..." instead of
                            > "/usr/local/...". On ubuntu (and probably on other linux distros as
                            > well) python3 is installed in /usr/local while python2 is installed
                            > in /usr.

                            That's not the case at all[0]. No distribution package should install
                            to /usr/local as that's a reserved directory structure for the system
                            administrator[1]. If your Python3 install is in /usr/local, then whoever
                            admins that system installed it there.

                            > The attached patch calls Py_SetPythonHome with PYTHON3_PREFIX
                            > defined by configure.
                            > This solves the problem.

                            This does make sense for supporting people who have installed Python
                            outside of the standard paths, though. This should probably be done for
                            Python2.x as well.

                            [0]: http://packages.ubuntu.com/maverick/i386/python3.1/filelist
                            [1]: http://www.pathname.com/fhs/pub/fhs-2.3.html#USRLOCALLOCALHIERARCHY
                            --
                            James
                            GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan@...>
                          • James Vega
                            ... Would it make more sense to use Py_GetPrefix() or Py_GetExecPrefix() as the input to Py_SetPythonHome() instead? -- James GPG Key: 1024D/61326D40
                            Message 13 of 16 , Nov 7, 2010
                              On Sun, Nov 07, 2010 at 01:53:05PM -0500, James Vega wrote:
                              > On Sun, Nov 07, 2010 at 06:04:35PM +0200, Roland Puntaier wrote:
                              > > The attached patch calls Py_SetPythonHome with PYTHON3_PREFIX
                              > > defined by configure.
                              > > This solves the problem.
                              >
                              > This does make sense for supporting people who have installed Python
                              > outside of the standard paths, though. This should probably be done for
                              > Python2.x as well.

                              Would it make more sense to use Py_GetPrefix() or Py_GetExecPrefix() as the
                              input to Py_SetPythonHome() instead?

                              --
                              James
                              GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan@...>
                            • Philippe Vaucher
                              ... Why the hell happened there? Seems you replied to the wrong thread and managed to change the subject. Look at
                              Message 14 of 16 , Nov 7, 2010
                                > After installing vim7.3 on a ubuntu system, I had again the problem that
                                > import did not work for .so libraries in lib-dynload. I found that
                                > sys.path was initialized with "/usr/..." instead of "/usr/local/...". On
                                > ubuntu (and probably on other linux distros as well) python3 is
                                > installed in /usr/local while python2 is installed in  /usr.
                                >
                                > The attached patch calls Py_SetPythonHome with PYTHON3_PREFIX defined by
                                > configure.
                                > This solves the problem.


                                Why the hell happened there? Seems you replied to the wrong thread and
                                managed to change the subject.

                                Look at http://groups.google.com/group/vim_dev/browse_thread/thread/6972ae8d369cf671/a183f61488f3006c#

                                Philippe

                                --
                                You received this message from the "vim_dev" maillist.
                                Do not top-post! Type your reply below the text you are replying to.
                                For more information, visit http://www.vim.org/maillist.php
                              • Roland Puntaier
                                ... OK, maybe I didn t use apt-get. Nevertheless, as most original sources do, python 3.x defaults to /usr/local prefix. And anyway, for the problem, it
                                Message 15 of 16 , Nov 7, 2010
                                  James Vega wrote:
                                  > On Sun, Nov 07, 2010 at 06:04:35PM +0200, Roland Puntaier wrote:
                                  >
                                  >> Hi Bram,
                                  >>
                                  >> After installing vim7.3 on a ubuntu system, I had again the problem
                                  >> that import did not work for .so libraries in lib-dynload. I found
                                  >> that sys.path was initialized with "/usr/..." instead of
                                  >> "/usr/local/...". On ubuntu (and probably on other linux distros as
                                  >> well) python3 is installed in /usr/local while python2 is installed
                                  >> in /usr.
                                  >>
                                  >
                                  > That's not the case at all[0]. No distribution package should install
                                  > to /usr/local as that's a reserved directory structure for the system
                                  > administrator[1]. If your Python3 install is in /usr/local, then whoever
                                  > admins that system installed it there.
                                  >
                                  OK, maybe I didn't use apt-get. Nevertheless, as most original sources do,
                                  python 3.x defaults to /usr/local prefix. And anyway, for the problem,
                                  it doesn't matter how it got there.
                                  >> The attached patch calls Py_SetPythonHome with PYTHON3_PREFIX
                                  >> defined by configure.
                                  >> This solves the problem.
                                  >>
                                  >
                                  > This should probably be done for Python2.x as well.
                                  >
                                  Yeah, you are right. I will send a new patch soon.

                                  --
                                  You received this message from the "vim_dev" maillist.
                                  Do not top-post! Type your reply below the text you are replying to.
                                  For more information, visit http://www.vim.org/maillist.php
                                • Roland Puntaier
                                  ... Sorry Philippe, Using Thunderbird I ve chosen an arbitrary message from vim_dev and changed subject and body. I didn t know that the threading is done on
                                  Message 16 of 16 , Nov 7, 2010
                                    Philippe Vaucher wrote:
                                    >
                                    > Why the hell happened there? Seems you replied to the wrong thread and
                                    > managed to change the subject.
                                    >
                                    > Look at http://groups.google.com/group/vim_dev/browse_thread/thread/6972ae8d369cf671/a183f61488f3006c#
                                    >
                                    > Philippe
                                    >
                                    >
                                    Sorry Philippe,

                                    Using Thunderbird I've chosen an arbitrary message from vim_dev and
                                    changed subject and body.

                                    I didn't know that the threading is done on invisible email message
                                    fields other than the subject. I looked at the email source and found
                                    fields like References and In-Reply-To.

                                    There is always something new to learn. For my next message I'll take it
                                    into account.

                                    --
                                    You received this message from the "vim_dev" maillist.
                                    Do not top-post! Type your reply below the text you are replying to.
                                    For more information, visit http://www.vim.org/maillist.php
                                  Your message has been successfully submitted and would be delivered to recipients shortly.