Re: [PATCH] Fix some temp file issues

[Eli Zaretskii]

> Date: Sat, 8 Oct 2022 21:23:53 -0700
> Cc: bug-make@gnu.org
> From: Paul Eggert 
> 
> On 2022-10-08 21:19, Eli Zaretskii wrote:
> > I meant the "b" part, not the "+" part.  On systems where that changes
> > the bytestream written to the file, the change might require a
> > suitable change where we read that stuff.
> 
> If I understand things correctly the code was formerly using tmpfile 
> which does use "b", so I figured "b" was fine.

Ah, okay.  Then I guess there's no problem after all.

> Another way to think about it: GNU 'make' just writes text to the file. 
> On MS-Windows if you're writing text using "b" doesn't a later read by 
> another process work regardless of whether the read uses "b"?

If the difference is only in EOL format, yes.  But "b" has other
implications, such as reading beyond the first ^Z byte.  Although I
doubt that this could happen in this case.

Re: [PATCH] Fix some temp file issues

[Paul Eggert]

On 2022-10-08 21:19, Eli Zaretskii wrote:

I meant the "b" part, not the "+" part.  On systems where that changes
the bytestream written to the file, the change might require a
suitable change where we read that stuff.


If I understand things correctly the code was formerly using tmpfile 
which does use "b", so I figured "b" was fine.


Another way to think about it: GNU 'make' just writes text to the file. 
On MS-Windows if you're writing text using "b" doesn't a later read by 
another process work regardless of whether the read uses "b"?

Re: [PATCH] Fix some temp file issues

[Eli Zaretskii]

> Date: Sat, 8 Oct 2022 15:22:50 -0700
> Cc: bug-make@gnu.org
> From: Paul Eggert 
> 
> On 2022-10-08 00:14, Eli Zaretskii wrote:
> >> tmpfile uses "wb+" (POSIX requires this) and we should be consistent in
> >> all the paths that create temporary FILE *. The attached patch adds a
> >> comment about this.
> > I don't remember: where is this temporary file read?
> 
> It's read by a different process (fork+exec), so it doesn't matter for 
> 'make' now whether it uses "w+" or "w". It matters only for possible 
> future changes to 'make' later, assuming we ever change other parts of 
> 'make' to sometimes need read access via the same FILE *.

I meant the "b" part, not the "+" part.  On systems where that changes
the bytestream written to the file, the change might require a
suitable change where we read that stuff.

Re: [PATCH] Fix some temp file issues

[Paul Eggert]

On 2022-10-08 00:14, Eli Zaretskii wrote:

tmpfile uses "wb+" (POSIX requires this) and we should be consistent in
all the paths that create temporary FILE *. The attached patch adds a
comment about this.

I don't remember: where is this temporary file read?


It's read by a different process (fork+exec), so it doesn't matter for 
'make' now whether it uses "w+" or "w". It matters only for possible 
future changes to 'make' later, assuming we ever change other parts of 
'make' to sometimes need read access via the same FILE *.

[bug #63185] configure fails to detect getloadavg declaration on sun and aix.

[Dmitry Goncharov]

Follow-up Comment #1, bug #63185 (project make):

The latest configure from 4.3.90 fails to detect getloadavg declaration on
sun
and aix.


../src/job.c: In function ▒load_too_high▒:
../src/job.c:2103:7: warning: implicit declaration of function
▒getloadavg▒ [-Wimplicit-function-declaration]
 2103 |   if (getloadavg (, 1) != 1)
  |   ^~



$ grep GETLOADAVG src/config.h
#define GNULIB_TEST_GETLOADAVG 1
#define HAVE_DECL_GETLOADAVG 1
$


The change was introduced in dd24a4c1cfa7b6928ddb2bcd00a023f23aaaf440.


$ git sh --pretty=format:%H dd24a4c1cfa7b6928ddb2bcd00a023f23aaaf440  --
configure.ac src/job.c
dd24a4c1cfa7b6928ddb2bcd00a023f23aaaf440
diff --git a/configure.ac b/configure.ac
index 51817b2c..4a0d5301 100644
--- a/configure.ac
+++ b/configure.ac
@@ -437,6 +437,9 @@ AS_CASE([$host],
 AC_DEFINE_UNQUOTED([PATH_SEPARATOR_CHAR],['$PATH_SEPARATOR'],
 [Define to the character that separates directories in PATH.])

+AC_DEFINE_UNQUOTED([HAVE_DECL_GETLOADAVG],[$HAVE_DECL_GETLOADAVG],
+[Define to 1 if you have the declaration of 'getloadavg'.])
+
 # Include the Maintainer's Makefile section, if it's here.

 MAINT_MAKEFILE=/dev/null
diff --git a/src/job.c b/src/job.c
index 0c9054bd..402d409f 100644
--- a/src/job.c
+++ b/src/job.c
@@ -216,7 +216,7 @@ pid2str (pid_t pid)
   return pidstring;
 }

-#ifndef HAVE_GETLOADAVG
+#ifndef HAVE_DECL_GETLOADAVG
 int getloadavg (double loadavg[], int nelem);
 #endif


This is the relevant part of config.log

configure:7677: checking for getloadavg
configure:7677: gcc-11 -o conftest -Wall -Wextra -ggdb -m64
-DMAKE_MAINTAINER_MODE=1   conftest.c  >&5
configure:7677: $? = 0
configure:7677: result: yes
configure:8030: checking for sys/loadavg.h
configure:8030: gcc-11 -c -Wall -Wextra -ggdb -m64 -DMAKE_MAINTAINER_MODE=1 
conftest.c >&5
configure:8030: $? = 0
configure:8030: result: yes
configure:8042: checking whether getloadavg is declared
configure:8042: gcc-11 -c -Wall -Wextra -ggdb -m64 -DMAKE_MAINTAINER_MODE=1  
conftest.c >&5
configure:8042: $? = 0
configure:8042: result: yes
...
| #define HAVE_SYS_LOADAVG_H 1
| #define GNULIB_TEST_GETLOADAVG 1
...
ac_cv_func_getloadavg=yes
...
ac_cv_have_decl_getloadavg=yes
...
GETLOADAVG_LIBS=''
GL_COND_OBJ_GETLOADAVG_FALSE=''
GL_COND_OBJ_GETLOADAVG_TRUE='#'
...
GL_GNULIB_GETLOADAVG='1'
...
HAVE_DECL_GETLOADAVG='1'
...
#define HAVE_DECL_GETLOADAVG 1


Reverting the change in job.c fixes the issue.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63185] configure fails to detect getloadavg declaration on sun and aix.

[Dmitry Goncharov]

URL:
  

 Summary: configure fails to detect getloadavg declaration on
sun and aix.
 Project: make
   Submitter: dgoncharov
   Submitted: Sat 08 Oct 2022 08:26:14 PM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: SCM
Operating System: POSIX-Based
   Fixed Release: None
   Triage Status: None


___

Follow-up Comments:


---
Date: Sat 08 Oct 2022 08:26:14 PM UTC By: Dmitry Goncharov 
.







___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

Re: [PATCH] Fix some temp file issues

[Eli Zaretskii]

> Date: Fri, 7 Oct 2022 21:37:24 -0700
> Cc: bug-make@gnu.org
> From: Paul Eggert 
> 
> > I'd appreciate a more high-level description of the idea of the
> > change, in addition to the gory details.
> 
> I gave it a shot in the attached patch, which is an improved version of 
> the previous patch.

Thanks, it's more clear now.

> > This opens the file in binary mode where previously it wasn't; what is
> > the rationale for the change?  And why the "+" part?
> 
> tmpfile uses "wb+" (POSIX requires this) and we should be consistent in 
> all the paths that create temporary FILE *. The attached patch adds a 
> comment about this.

I don't remember: where is this temporary file read?  Because that
place where the file is read will most probably need to open the file
in binary mode and/or deal with that when it reads the file.