From: Costin Lupu <costin.lupu@cs.pub.ro>
To: Julien Grall <julien@xen.org>, xen-devel@lists.xenproject.org
Cc: Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 2/5] tools/libfsimage: Fix PATH_MAX redefinition error
Date: Wed, 28 Apr 2021 21:35:49 +0300 [thread overview]
Message-ID: <d29a7d0a-d83e-a437-d38f-d142a2f0d1a7@cs.pub.ro> (raw)
In-Reply-To: <9ef85c5b-b757-bbaa-be23-4ceb1d45cddd@xen.org>
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@cs.pub.ro>
>> ---
>> 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.
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.
So IMHO the current changes seem to be the most portable, but I'm open
to any suggestions.
[1]
https://github.com/jemalloc/jemalloc/blob/dev/include/jemalloc/internal/jemalloc_internal_decls.h#L76
>> #define MAX_LINK_COUNT 5 /* number of symbolic links
>> to follow */
>> /* made up, these are pointers into FSYS_BUF */
>> diff --git a/tools/libfsimage/reiserfs/fsys_reiserfs.c
>> b/tools/libfsimage/reiserfs/fsys_reiserfs.c
>> index 916eb15292..10ca657476 100644
>> --- a/tools/libfsimage/reiserfs/fsys_reiserfs.c
>> +++ b/tools/libfsimage/reiserfs/fsys_reiserfs.c
>> @@ -284,7 +284,9 @@ struct reiserfs_de_head
>> #define S_ISDIR(mode) (((mode) & 0170000) == 0040000)
>> #define S_ISLNK(mode) (((mode) & 0170000) == 0120000)
>> +#ifndef PATH_MAX
>> #define PATH_MAX 1024 /* include/linux/limits.h */
>> +#endif
>> #define MAX_LINK_COUNT 5 /* number of symbolic links to follow */
>> /* The size of the node cache */
>>
>
next prev parent reply other threads:[~2021-04-28 18:36 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-27 12:05 [PATCH 0/5] Fix redefinition errors for toolstack libs Costin Lupu
2021-04-27 12:05 ` [PATCH 1/5] tools/debugger: Fix PAGE_SIZE redefinition error Costin Lupu
2021-04-29 19:58 ` Tim Deegan
2021-04-30 11:36 ` Costin Lupu
2021-04-30 18:45 ` Tim Deegan
2021-04-30 19:33 ` Costin Lupu
2021-04-27 12:05 ` [PATCH 2/5] tools/libfsimage: Fix PATH_MAX " Costin Lupu
2021-04-28 9:04 ` Julien Grall
2021-04-28 18:35 ` Costin Lupu [this message]
2021-04-29 11:39 ` Julien Grall
2021-04-27 12:05 ` [PATCH 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE " Costin Lupu
2021-04-28 9:03 ` Julien Grall
2021-04-28 18:27 ` Costin Lupu
2021-04-29 11:29 ` Julien Grall
2021-04-27 12:05 ` [PATCH 4/5] tools/libs/gnttab: " Costin Lupu
2021-04-27 12:05 ` [PATCH 5/5] tools/ocaml: Fix redefinition errors Costin Lupu
2021-04-28 12:34 ` [PATCH 0/5] Fix redefinition errors for toolstack libs Christian Lindig
2021-04-28 18:37 ` Costin Lupu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d29a7d0a-d83e-a437-d38f-d142a2f0d1a7@cs.pub.ro \
--to=costin.lupu@cs.pub.ro \
--cc=iwj@xenproject.org \
--cc=julien@xen.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).