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

Re: [PATCH 2/5] tools/libfsimage: Fix PATH_MAX redefinition error



Hi Costin,

On 28/04/2021 19:35, Costin Lupu wrote:
On 4/28/21 12:04 PM, Julien Grall wrote:


On 27/04/2021 13:05, Costin Lupu wrote:
If PATH_MAX is already defined in the system (e.g. in
/usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror.

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
---
   tools/libfsimage/ext2fs/fsys_ext2fs.c     | 2 ++
   tools/libfsimage/reiserfs/fsys_reiserfs.c | 2 ++
   2 files changed, 4 insertions(+)

diff --git a/tools/libfsimage/ext2fs/fsys_ext2fs.c
b/tools/libfsimage/ext2fs/fsys_ext2fs.c
index a4ed10419c..5ed8fce90e 100644
--- a/tools/libfsimage/ext2fs/fsys_ext2fs.c
+++ b/tools/libfsimage/ext2fs/fsys_ext2fs.c
@@ -278,7 +278,9 @@ struct ext4_extent_header {
     #define EXT2_SUPER_MAGIC      0xEF53    /* include/linux/ext2_fs.h */
   #define EXT2_ROOT_INO              2    /* include/linux/ext2_fs.h */
+#ifndef PATH_MAX
   #define PATH_MAX                1024    /* include/linux/limits.h */
+#endif

Can we drop it completely and just rely on limits.h?


One problem here is that the system limits.h header doesn't necessarily
include linux/limits.h, which would mean we would have to include
linux/limits.h. But this is problematic for other systems such as BSD.

That's annoying :).


I had a look on a FreeBSD source tree to see how this is done there. It
seems that there are lots of submodules, apps and libs that redefine
PATH_MAX in case it wasn't defined before so the changes introduced by
the current patch seem to be very popular. Another clean approach I saw
was for jemalloc [1] which includes unistd.h. They redefine PATH_MAX
only for MS C compiler, but AFAIK we don't need that.

I am not aware of anyone using MS C compiler to build the tools.


So IMHO the current changes seem to be the most portable, but I'm open
to any suggestions.

Right, this is the good thing of your approach. I can't see a better solution if the system limits.h doesn't always define PATH_MAX. So:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.