Skip to content

vlogger: a couple of bugfixes.#2

Closed
CMB wants to merge 1 commit intovoid-linux:masterfrom
CMB:cmb-vlogger
Closed

vlogger: a couple of bugfixes.#2
CMB wants to merge 1 commit intovoid-linux:masterfrom
CMB:cmb-vlogger

Conversation

@CMB
Copy link
Copy Markdown
Contributor

@CMB CMB commented Jun 22, 2018

  • Don't store the return value of getopt in a char. Storing in a char
    and comparing against -1 breaks on ARM, where char is unsigned.
  • The line argument to getline should point at a NULL char *. Otherwise,
    getline will treat *line as a pointer to an allocated buffer. With
    a little extra work, we could reuse the buffer, but always calling getline
    with *line == NULL is safe.

* Don't store the return value of getopt in a char.  Storing in a char
  and comparing against -1 breaks on ARM, where char is unsigned.
* The line argument to getline should point at a NULL char *.  Otherwise,
  getline will treat *line as a pointer to an allocated buffer.  With
  a little extra work, we could reuse the buffer, but always calling getline
  with *line == NULL is safe.
@Duncaen
Copy link
Copy Markdown
Member

Duncaen commented Jun 22, 2018

Initializing line to NULL is good, but I don't think its necessary to free it every iteration, getline can grow the buffer, I don't think its necessary to shrink it.
And can you add a space between char and and *p.

@Duncaen Duncaen closed this in a2320e5 Jun 23, 2018
@CMB CMB deleted the cmb-vlogger branch June 23, 2018 00:34
robang74 pushed a commit to robang74/void-runit that referenced this pull request Aug 19, 2025
* Don't store the return value of getopt in a char.  Storing in a char
  and comparing against -1 breaks on ARM, where char is unsigned.
* The line argument to getline should point at a NULL char *.  Otherwise,
  getline will treat *line as a pointer to an allocated buffer.  With
  a little extra work, we could reuse the buffer, but always calling getline
  with *line == NULL is safe.

Closes: void-linux#2 [via git-merge-pr]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants