I'm not sure I understand the point of the check fgets(...) == NULL || fgets(...) != NULL? I don't understand why there was a while loop in the first place (maybe because fgets stops reading upon a newline? But even then the old code looks incorrect).

Would the following fix also work in the situations you are describing?

buffer[0] = '\0';
FILE * output = ...
if (output) {
  fgets(...);
  buffer[FILE_DIALOG_MAX_BUFFER - 1] = '\0';
}

There are several potential failure cases:

1. popen() fails and sets output to NULL;
2. output is empty (i.e. the user canceled the file dialog without picking a file);
3. the pipe breaks while reading the filename (so that fgets() fails and sets an error flag);
4. the chosen file is too long to fit in buffer (and hence will become invalid if naively truncated to FILE_DIALOG_MAX_BUFFER-1 characters.)

In all these cases the buffer needs to be cleared of its contents so that the function returns an empty string (per the specification in the header).

Your version handles case 1 only. Mine handles 1, 2, 4, and almost 3 (I just noticed that there is a corner case I missed: the user picks an overlong file, which is partially read by the first fgets() call, and then the pipe breaks for some reason before the second fgets() call, whose NULL return is incorrectly interpreted as an EOF. This (presumably, very rare) eventuality can be detected by appending || ferror(output) to the conditional expression.

Ok I see. Cramming all those checks in a single if() statement makes it difficult to follow the flow. Also, with your double fgets() you are reading the stream multiple times, which is a bit weird (or maybe I'm still missing something here?).

How about the following:

buffer[0] = '\0';
buffer[FILE_DIALOG_MAX_BUFFER - 1] = 'x'; // Initialize last character with a char != '\0'
FILE * output = ...
if (output) {
  auto ret = fgets(buffer, FILE_DIALOG_MAX_BUFFER, output);
  if (ret == NULL || ferror(output)) {
    // I/O error
    buffer[0] = '\0';
  }
  if (buffer[FILE_DIALOG_MAX_BUFFER - 1] == '\0') {
    // File name too long, buffer has been filled, so we return empty string instead
    buffer[0] = '\0';
  }
}

Sure that seems fine (other than disallowing names of length exactly FILE_DIALOG_MAX_BUFFER - 1, which may be unimportant).

We can replace FILE_DIALOG_MAX_BUFFER by 1024+1 if you prefer :D