[BUG] pgindent not working properly

[zhjwpku]

Describe the bug

I'm playing with v1.4.2, there seems still some unexpected behaviors, take src/backend/utils/mmgr/mcxt.c as an example, after format with the extension, there are some format issues, e.g. function return void * changed to void *, it seems that in runPgindentInternal, you are downloading typedefs.list from internet because pg_bsd_indent directory doesn't contains it.

I see you comment that to use pg_bsd_indent directly instead of pgindent for some reason, but in my case(out of tree build) the issue happened. The following command works in my case, hope this can give you some hints.

src/tools/pgindent/pgindent --indent /home/postgres/build/src/tools/pg_bsd_indent/pg_bsd_indent /home/postgres/postgres/src/backend/utils/mmgr/mcxt.c

I'd suggest keeping the PgindentPath setting, when user set it, call pgindent instead of pg_bsd_indent directly, what do you think? @ashenBlade

[ashenBlade]

Hi! Thanks for request. I looked as sources on pgindent again and found that i have missed post_indent function that performs some clean ups (including single space before *).
I will fix this soon.

And the reason why I do not use pgindent directly - because of many backup files *.BAK that it creates. If pg_bsd_indent fails (sometimes it happens randomly) that file will not get deleted. I do think this logic will be changed it near future, so just copying the code logic is a good compromise (i will add this note to source code).

[zhjwpku]

Hi! Thanks for request. I looked as sources on pgindent again and found that i have missed post_indent function that performs some clean ups (including single space before *). I will fix this soon.

And the reason why I do not use pgindent directly - because of many backup files *.BAK that it creates. If pg_bsd_indent fails (sometimes it happens randomly) that file will not get deleted. I do think this logic will be changed it near future, so just copying the code logic is a good compromise (i will add this note to source code).

Ok, reasonable. Looking forward to testing the fixed version.

[ashenBlade]

Fixed this in new 1.4.3 release. If it helped, then close the issue, please.

[zhjwpku]

Fixed this in new 1.4.3 release. If it helped, then close the issue, please.

Hi, I just did a quick test, it works for the * case, but there are still some unexpected behavior, like if I format the mcxt.c file, the following happens:

diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 70d33226cb..9e9d09efb2 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -200,12 +200,12 @@ GetMemoryChunkMethodID(const void *pointer)
        Assert(pointer == (const void *) MAXALIGN(pointer));
 
        /* Allow access to the uint64 header */
-       VALGRIND_MAKE_MEM_DEFINED((char *) pointer - sizeof(uint64), sizeof(uint64));
+       VALGRIND_MAKE_MEM_DEFINED((char *) pointer -sizeof(uint64), sizeof(uint64));
 
-       header = *((const uint64 *) ((const char *) pointer - sizeof(uint64)));
+       header = *((const uint64 *) ((const char *) pointer -sizeof(uint64)));
 
        /* Disallow access to the uint64 header */
-       VALGRIND_MAKE_MEM_NOACCESS((char *) pointer - sizeof(uint64), sizeof(uint64));
+       VALGRIND_MAKE_MEM_NOACCESS((char *) pointer -sizeof(uint64), sizeof(uint64));
 
        return (MemoryContextMethodID) (header & MEMORY_CONTEXT_METHODID_MASK);
 }
@@ -222,12 +222,12 @@ GetMemoryChunkHeader(const void *pointer)
        uint64          header;
 
        /* Allow access to the uint64 header */
-       VALGRIND_MAKE_MEM_DEFINED((char *) pointer - sizeof(uint64), sizeof(uint64));
+       VALGRIND_MAKE_MEM_DEFINED((char *) pointer -sizeof(uint64), sizeof(uint64));
 
-       header = *((const uint64 *) ((const char *) pointer - sizeof(uint64)));
+       header = *((const uint64 *) ((const char *) pointer -sizeof(uint64)));
 
        /* Disallow access to the uint64 header */
-       VALGRIND_MAKE_MEM_NOACCESS((char *) pointer - sizeof(uint64), sizeof(uint64));
+       VALGRIND_MAKE_MEM_NOACCESS((char *) pointer -sizeof(uint64), sizeof(uint64));
 
        return header;
 }

[ashenBlade]

Hi, after some hours of searching i found that not only source code must be processed, but also typedefs.list (add and remove some entries).

Experiments shew that it was root cause of problem. I will fix this soon.

[ashenBlade]

I created prerelease version 1.4.4 with fixes. If this helps then tell me, i will publish it.

[zhjwpku]

I created prerelease version 1.4.4 with fixes. If this helps then tell me, i will publish it.

It works, I will close this issue with this comment, thanks ;)