Skip to contentSkip to conversation

cbootimage: Add 'b' (binary) flag when using fopen to open a binary file.

PATCH In review Scott Duplichan 2 participants 4 messages Dec 14, 2014 – Dec 15, 2014 1 message not in the archive Tegra architecture support

lore
Share & tools
lore

Scott Duplichan

author
#0 lore raw

PATCH 1/1 cbootimage: Add 'b' (binary) flag when using fopen to open a binary file.

Add 'b' (binary) flag when using fopen to open a binary file. This keeps Windows from expanding \n to \r\n and interpreting <ctrl>z as end of file. The change is to support a Windows hosted coreboot build environment.

Signed-off-by Scott Duplichan <[email protected]>

---

src/set.c +1 -1
index 0af6686..e8ae3dd 100644
@@ -53,7 +53,7 @@ read_from_image(char	*filename,
 	FILE *fp;
 	struct stat stats;
 
-	fp = fopen(filename, "r");
+	fp = fopen(filename, "rb");
 	if (fp == NULL) {
 		result = 1;
 		return result;
#1 lore raw
Quoted 4 lines · Scott Duplichan

Add 'b' (binary) flag when using fopen to open a binary file. This keeps Windows from expanding \n to \r\n and interpreting <ctrl>z as end of file. The change is to support a Windows hosted coreboot build environment.

This seems fine; I'll apply it in just a second.

It seems worth being explicit on all fopens re: b-vs-t. To that end, perhaps you could update the following too?

Quoted 3 lines

./src/cbootimage.c:161: context->config_file = fopen(argv[optind++], "r");
./src/cbootimage.c:218: context.raw_file = fopen(context.output_image_filename, "w+");
./src/data_layout.c:1049: fp = fopen(context->input_image_filename, "r");

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#3 lore raw

Stephen Warren [mailto:[email protected]] wrote:

]Sent: Monday, December 15, 2014 11:28 AM
]To: Scott Duplichan
]Cc: [email protected]
]Subject: Re: [cbootimage PATCH] cbootimage: Add 'b' (binary) flag when using fopen to open a binary file. ]
]On 12/14/2014 09:59 AM, Scott Duplichan wrote:
]> Add 'b' (binary) flag when using fopen to open a binary file.
]> This keeps Windows from expanding \n to \r\n and interpreting
]> <ctrl>z as end of file. The change is to support a Windows
]> hosted coreboot build environment.
]
]This seems fine; I'll apply it in just a second.

Hello Stephen,

Thanks a lot for applying the patch.

]It seems worth being explicit on all fopens re: b-vs-t. To that end, ]perhaps you could update the following too?
]
]> ./src/cbootimage.c:161: context->config_file = fopen(argv[optind++], "r");
]> ./src/cbootimage.c:218: context.raw_file = fopen(context.output_image_filename, "w+");
]> ./src/data_layout.c:1049: fp = fopen(context->input_image_filename, "r");

I thought about this too. Microsoft defines a 't' flag for text mode. So if fopen is called with flags "rt", the Microsoft libraries will understand it as read, text mode. But the 't' flag is not part of C99 or posix. C99 explicitly states "r" is "open text file for reading" and "rb" as "open binary file for reading". Though the 't' flag will probably be harmlessly ignored by non-Microsoft libraries, it is probably best to avoid it since it is Microsoft-specific. I think the only time the 't' flag is needed is if the Microsoft function _set_fmode has been used to change the default open mode from text to binary.

Thanks,
Scott

#4 lore raw
Quoted 26 lines · Scott Duplichan

Stephen Warren [mailto:[email protected]] wrote:

]Sent: Monday, December 15, 2014 11:28 AM
]To: Scott Duplichan
]Cc: [email protected]
]Subject: Re: [cbootimage PATCH] cbootimage: Add 'b' (binary) flag when using fopen to open a binary file. ]
]On 12/14/2014 09:59 AM, Scott Duplichan wrote:
]> Add 'b' (binary) flag when using fopen to open a binary file.
]> This keeps Windows from expanding \n to \r\n and interpreting
]> <ctrl>z as end of file. The change is to support a Windows
]> hosted coreboot build environment.
]
]This seems fine; I'll apply it in just a second.

Hello Stephen,

Thanks a lot for applying the patch.

]It seems worth being explicit on all fopens re: b-vs-t. To that end, ]perhaps you could update the following too?
]
]> ./src/cbootimage.c:161: context->config_file = fopen(argv[optind++], "r");
]> ./src/cbootimage.c:218: context.raw_file = fopen(context.output_image_filename, "w+");
]> ./src/data_layout.c:1049: fp = fopen(context->input_image_filename, "r");

Quoted 9 lines

I thought about this too. Microsoft defines a 't' flag for text mode. So if fopen is called with flags "rt", the Microsoft libraries will understand it as read, text mode. But the 't' flag is not part of C99 or posix. C99 explicitly states "r" is "open text file for reading" and "rb" as "open binary file for reading". Though the 't' flag will probably be harmlessly ignored by non-Microsoft libraries, it is probably best to avoid it since it is Microsoft-specific. I think the only time the 't' flag is needed is if the Microsoft function _set_fmode has been used to change the default open mode from text to binary.

Indeed you're right.