Re: 32 bit build failure (smb, narrowing)

[Kevin Ottens]

Hello,

Sorry for the delay, end of last week has been a bit hectic... I see you 
already made a fix. Thanks a lot!

Regards.

On Friday, 8 March 2024 08:23:29 CET Rene Engelhard wrote:
> Hi,
> 
> see
> https://buildd.debian.org/status/fetch.php?pkg=libreoffice=i386=4%3
> A24.2.2~rc1-1=1709881487=1:
> 
> /<>/sal/osl/unx/file.cxx: In function ‘void
> osl_file_adjustLockFlags(const rtl::OString&, int*, sal_uInt32*)’:
> /<>/sal/osl/unx/file.cxx:71:26: error: narrowing conversion
> of ‘4283649346’ from ‘unsigned int’ to ‘int’ [-Wnarrowing]
> 71 | #define CIFS_SUPER_MAGIC 0xFF534D42
> 
>|  ^~
> 
> /<>/sal/osl/unx/file.cxx:795:14: note: in expansion of
> macro ‘CIFS_SUPER_MAGIC’
> 
>795 | case CIFS_SUPER_MAGIC:
>|  ^~~~
> 
> /<>/sal/osl/unx/file.cxx:72:26: error: narrowing conversion
> of ‘4266872130’ from ‘unsigned int’ to ‘int’ [-Wnarrowing]
> 72 | #define SMB2_SUPER_MAGIC 0xFE534D42
> 
>|  ^~
> 
> /<>/sal/osl/unx/file.cxx:796:14: note: in expansion of
> macro ‘SMB2_SUPER_MAGIC’
> 
>796 | case SMB2_SUPER_MAGIC:
>|  ^~~~
> 
> make[2]: *** [/<>/solenv/gbuild/LinkTarget.mk:340:
> /<>/workdir/CxxObject/sal/osl/unx/file.o] Error 1
> 
> This is due to
> 
> commit a8814b5921676b1c01a19b0af243712c55fb0307
> Author: Kevin Ottens 
> Date:   Fri Feb 2 15:39:36 2024 +0100
> 
>  tdf#55004 Fix backup copy creation for files on mounted samba shares
> 
>  There is an unfortunate interaction between file locking and backup
>  creation at save time.
> 
>  openFilePath has logic to lock a file when opening. This goes through
>  fcntl to set a write lock on the file. Later on, when the user wants to
> save changes, a backup copy might be created (very likely now since this
>  is the defaults in the settings). To create this backup, the file is
>  opened again for reading. Unfortunately this open call fails due to the
> lock (even though it is a write lock).
> 
>  This commit changes the behavior. osl_file_adjustLockFlags now
> checks if
>  the file is on a mounted samba share. If that's the case we force the
>  osl_File_OpenFlag_NoLock flag. No issue is then exhibited at backup
>  creation, allowing the save to proceed properly.
> 
>  Change-Id: Ieab252f9f68598834e13339fc5fcea440f0a4c2f
>  Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162935
>  Tested-by: Jenkins
>  Reviewed-by: Stephan Bergmann 
>  (cherry picked from commit 63efbc8ad8aae12b54e649c1495d1233c1a9b33f)
>  Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163549
> 
> Can you have a look, please? I had a brief one, but that is a simple
> define which shoudln't break. Then the switch does
>  switch (aFileStatFs.f_type) {
> which uses
>  struct statfs aFileStatFs;
> 
> So it probably is a type difference in the kernel struct already for 32
> vs 64 bit?
> 
> Regards,
> 
> Rene


-- 
Kévin Ottens
kevin.ott...@enioka.com
+33 7 57 08 95 13

Re: 32 bit build failure (smb, narrowing)

[Stephan Bergmann]

On 3/8/24 08:23, Rene Engelhard wrote:
see 
https://buildd.debian.org/status/fetch.php?pkg=libreoffice=i386=4%3A24.2.2~rc1-1=1709881487=1:


/<>/sal/osl/unx/file.cxx: In function ‘void 
osl_file_adjustLockFlags(const rtl::OString&, int*, sal_uInt32*)’:
/<>/sal/osl/unx/file.cxx:71:26: error: narrowing conversion 
of ‘4283649346’ from ‘unsigned int’ to ‘int’ [-Wnarrowing]

    71 | #define CIFS_SUPER_MAGIC 0xFF534D42
   |  ^~
/<>/sal/osl/unx/file.cxx:795:14: note: in expansion of 
macro ‘CIFS_SUPER_MAGIC’

   795 | case CIFS_SUPER_MAGIC:
   |  ^~~~
/<>/sal/osl/unx/file.cxx:72:26: error: narrowing conversion 
of ‘4266872130’ from ‘unsigned int’ to ‘int’ [-Wnarrowing]

    72 | #define SMB2_SUPER_MAGIC 0xFE534D42
   |  ^~
/<>/sal/osl/unx/file.cxx:796:14: note: in expansion of 
macro ‘SMB2_SUPER_MAGIC’

   796 | case SMB2_SUPER_MAGIC:
   |  ^~~~
make[2]: *** [/<>/solenv/gbuild/LinkTarget.mk:340: 
/<>/workdir/CxxObject/sal/osl/unx/file.o] Error 1


see  "Blind fix for 
Linux 32-bit builds"

Re: 32 bit build failure (smb, narrowing)

[Rene Engelhard]

Hi,

Am 08.03.24 um 08:23 schrieb Rene Engelhard:
Can you have a look, please? I had a brief one, but that is a simple 
define which shoudln't break. Then the switch does

     switch (aFileStatFs.f_type) {
which uses
     struct statfs aFileStatFs;

So it probably is a type difference in the kernel struct already for 32 
vs 64 bit?


Obviously I am too blind, there is even a if comparison before:

/* get filesystem info */
struct statfs aFileStatFs;
if (statfs(path.getStr(), ) < 0)

But as gcc complains about the (usage of the) define I don't guess 
that's it?


Regards,

Rene