Hey.
On Fri, Jun 16, 2023 at 5:04 PM Corinna Vinschen
<corinna-cyg...@cygwin.com> wrote:
> Oh well. Now that I see it in real life, my idea to use the entire
> expression inline wasn't that great, it seems...
^^
> I didn't want to keep MAX_EA_NAME_LEN because now that we have an
> official name for the value, having an unofficial name using a different
> naming convention is a bit weird.
>
> On the other hand, having a macro for the expression certainly looks
> much cleaner. Also, only one place to change (should a change ever be
> necessary).
Does both make sense.
> Sorry about that.
No worries :-)
> What do you think about something like _XATTR_NAME_MAX_ONDISK_?
Really with trailing/leading underscores? If you try to keep it out of
the "official namespace", then I'd would perhaps make more sense to
mark this as being cygwin specific like CYGWIN_XATTR_NAME_MAX_ONDISK
or so?
Also - may be nitpicking - but storage is not really guaranteed to be
a disk anymore. Maybe ONSTORAGE instead? But admittedly ONDISK sounds
more common ("on disk format", etc.).
> I can also just push the patches and we discuss this further afterwards,
> your call.
Well you know the naming convention used in your code much better than I do.
Attached patches use _XATTR_NAME_MAX_ONDISK_ as you proposed.
Just pick whichever name you like best, and either tell me and I
provide a new patch, or just sed 's/_XATTR_NAME_MAX_ONDISK_/foobar/g'
(+ maybe align text wrapping of comments if necessary).
Thanks,
Philippe
From 82e2ff6f52d7401210247ae396ce3f1f115d93f0 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon <philc...@gmail.com>
Date: Tue, 30 May 2023 13:16:18 +0200
Subject: [PATCH 1/2] Cygwin: export XATTR_{NAME,SIZE,LIST}_MAX
These are used for example by CPython.
Signed-off-by: Philippe Cerfon <philc...@gmail.com>
Signed-off-by: Corinna Vinschen <cori...@vinschen.de>
---
winsup/cygwin/include/cygwin/limits.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/winsup/cygwin/include/cygwin/limits.h b/winsup/cygwin/include/cygwin/limits.h
index aefc7c7bd..ea3e2836a 100644
--- a/winsup/cygwin/include/cygwin/limits.h
+++ b/winsup/cygwin/include/cygwin/limits.h
@@ -56,4 +56,11 @@ details. */
#define __PATH_MAX 4096
#define __PIPE_BUF 4096
+/* XATTR_NAME_MAX is the maximum XATTR name length excluding the null
+ * terminator. Since only XATTRs in the `user' namespace are allowed and the
+ * `user.' prefix is not stored, the maximum is increased by 5. */
+#define XATTR_NAME_MAX 260
+#define XATTR_SIZE_MAX 65536
+#define XATTR_LIST_MAX 65536
+
#endif /* _CYGWIN_LIMITS_H__ */
--
2.40.1
From 4c94f80fc817bee0e59da39397391cf2409cf029 Mon Sep 17 00:00:00 2001
From: Philippe Cerfon <philc...@gmail.com>
Date: Tue, 6 Jun 2023 02:52:49 +0200
Subject: [PATCH 2/2] Cygwin: use new XATTR_{NAME,SIZE}_MAX instead of
MAX_EA_{NAME,VALUE}_LEN
Signed-off-by: Philippe Cerfon <philc...@gmail.com>
---
winsup/cygwin/ntea.cc | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/winsup/cygwin/ntea.cc b/winsup/cygwin/ntea.cc
index a400fcb2b..70815649c 100644
--- a/winsup/cygwin/ntea.cc
+++ b/winsup/cygwin/ntea.cc
@@ -17,9 +17,11 @@ details. */
#include "tls_pbuf.h"
#include <stdlib.h>
#include <attr/xattr.h>
+#include <cygwin/limits.h>
-#define MAX_EA_NAME_LEN 256
-#define MAX_EA_VALUE_LEN 65536
+/* On storage the `user.` prefix is not included but the terminating null byte
+ is needed.*/
+#define _XATTR_NAME_MAX_ONDISK_ (XATTR_NAME_MAX - strlen("user.") + 1)
/* At least one maximum sized entry fits.
CV 2014-04-04: NtQueryEaFile function chokes on buffers bigger than 64K
@@ -27,13 +29,13 @@ details. */
on a remote share, at least on Windows 7 and later.
In theory the buffer should have a size of
- sizeof (FILE_FULL_EA_INFORMATION) + MAX_EA_NAME_LEN
- + MAX_EA_VALUE_LEN
+ sizeof (FILE_FULL_EA_INFORMATION) + _XATTR_NAME_MAX_ONDISK_
+ + XATTR_SIZE_MAX
(65804 bytes), but we're opting for simplicity here, and
a 64K buffer has the advantage that we can use a tmp_pathbuf
buffer, rather than having to alloca 64K from stack. */
-#define EA_BUFSIZ MAX_EA_VALUE_LEN
+#define EA_BUFSIZ XATTR_SIZE_MAX
#define NEXT_FEA(p) ((PFILE_FULL_EA_INFORMATION) (p->NextEntryOffset \
? (char *) p + p->NextEntryOffset : NULL))
@@ -55,7 +57,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
returns the last EA entry of the file infinitely. Even utilizing the
optional EaIndex only helps marginally. If you use that, the last
EA in the file is returned twice. */
- char lastname[MAX_EA_NAME_LEN];
+ char lastname[_XATTR_NAME_MAX_ONDISK_];
__try
{
@@ -95,7 +97,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
__leave;
}
- if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
+ if ((nlen = strlen (name)) >= _XATTR_NAME_MAX_ONDISK_)
{
set_errno (EINVAL);
__leave;
@@ -197,7 +199,7 @@ read_ea (HANDLE hdl, path_conv &pc, const char *name, char *value, size_t size)
/* For compatibility with Linux, we always prepend "user." to
the attribute name, so effectively we only support user
attributes from a application point of view. */
- char tmpbuf[MAX_EA_NAME_LEN * 2];
+ char tmpbuf[_XATTR_NAME_MAX_ONDISK_ * 2];
char *tp = stpcpy (tmpbuf, "user.");
stpcpy (tp, fea->EaName);
/* NTFS stores all EA names in uppercase unfortunately. To
@@ -297,7 +299,7 @@ write_ea (HANDLE hdl, path_conv &pc, const char *name, const char *value,
/* Skip "user." prefix. */
name += 5;
- if ((nlen = strlen (name)) >= MAX_EA_NAME_LEN)
+ if ((nlen = strlen (name)) >= _XATTR_NAME_MAX_ONDISK_)
{
set_errno (EINVAL);
__leave;
--
2.40.1
