git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] gc: use temporary file for editing crontab

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 29, 2022, 06:46 UTC
Message-ID
<xmqqmtbnr1hc.fsf@gitster.g>
In-Reply-To
<20220828214143.754759-1-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> While cron is specified by POSIX, there are a wide variety of
> implementations in use.
I would innsert: 
    "git maintenance" assumes that the "crontab" command can be fed
    from its standard input the new contents and the syntax to do so
    is not to have any filename argument, as POSIX describes.
    However,
here and downcase "O" in "On FreeBSD".
> On FreeBSD, the cron implementation
> requires a file name argument: if the user wants to edit standard
> input, they must specify "-".  
> However, this notation is not
> specified by POSIX, allowing the possibility that making such a
> change may break other, less common implementations.
And to avoid two However's in a row, perhaps
    Unfortunately, POSIX systems do not have to interpret "-" on the
    command line of crontab as a request to read from the standard
    input.  Blindly adding "-" on the command line would not work as
    a general solution.
Show 17 quoted lines
> Since POSIX tells us that cron must accept a file name argument, let's
> solve this problem by specifying a temporary file instead.  This will
> ensure that we work with the vast majority of implementations.
>
> Note that because delete_tempfile closes the file for us, we should not
> call fclose here on the handle, since doing so will introduce a double
> free.
>
> Reported-by: Renato Botelho <garga@FreeBSD.org>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> Changes from v1:
>
> * Use `goto out;` in additional places.
> * Fix broken test.
> * Use `delete_tempfile`.
> * Improve commit message to mention `fclose` rationale.
Yup.  All nicely done.
>  builtin/gc.c            | 39 +++++++++++++++++++++++----------------
>  t/helper/test-crontab.c |  4 ++--
>  2 files changed, 25 insertions(+), 18 deletions(-)
Will queue.  Thanks.
Previous: brian m. carlsonNext: Renato Botelho
Message 16 of 19 in “git maintenance broken on FreeBSD”
  1. Renato BotelhoAug 12, 2022
  2. Đoàn Trần Công DanhAug 12, 2022
  3. Todd ZullingerAug 13, 2022
  4. Junio C HamanoAug 13, 2022
  5. Đoàn Trần Công DanhAug 13, 2022
  6. Junio C HamanoAug 13, 2022
  7. brian m. carlsonAug 13, 2022
  8. Derrick StoleeAug 15, 2022
  9. Junio C HamanoAug 15, 2022
  10. gc: use temporary file for editing crontabbrian m. carlson, Aug 23, 2022
  11. Johannes SchindelinAug 23, 2022
  12. Derrick StoleeAug 23, 2022
  13. brian m. carlsonAug 23, 2022
  14. Junio C HamanoAug 24, 2022
  15. gc: use temporary file for editing crontabbrian m. carlson, Aug 28, 2022
  16. Junio C HamanoAug 29, 2022
  17. Renato BotelhoAug 29, 2022
  18. Derrick StoleeAug 30, 2022
  19. test-crontab: minor memory and error handling fixesJeff King, Aug 30, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.