Improve -S handling

PATCH In review Jerry Van Baren 3 participants 7 messages Apr 18, 2007 – Apr 19, 2007

lore

Share & tools

lore

Jerry Van Baren

author

2007-04-18 02:05 UTC #0 lore raw

PATCH 1/1 Improve -S handling

If the user requests extra space, pad out the blob (previously the unused
data was undefined).

Signed-off-by Gerald Van Baren <[email protected]>

---

Hi Jon, David, Milton,

This improves the -S option to pad out the blob with zeros when the user asks for extra space.

Best regards,
gvb

 flattree.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

flattree.c +34 -1

index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 	
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,

@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 
+	size = 0;
+
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
 			vi = &version_table[i];

@@ -384,10 +388,13 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 		 boot_cpuid_phys);
 
 	fwrite(&bph, vi->hdr_size, 1, f);
+	size += vi->hdr_size;
 
 	/* Align the reserve map to an 8 byte boundary */
-	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++)
+	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++) {
 		fputc(0, f);
+		size += 1;
+	}
 
 	/*
 	 * Reserve map entries.

@@ -396,9 +403,27 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 	 */
 	fwrite(reservebuf.val, reservebuf.len, 1, f);
 	fwrite(&termre, sizeof(termre), 1, f);
+	size += reservebuf.len + sizeof(termre);
 
 	fwrite(dtbuf.val, dtbuf.len, 1, f);
 	fwrite(strbuf.val, strbuf.len, 1, f);
+	size += dtbuf.len + strbuf.len;
+
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		int padlen = minsize - size;
+
+		if (padlen > 0) {
+			char *zeroes = calloc(padlen, 1);
+
+			if (zeroes != NULL) {
+				fwrite(zeroes, padlen, 1, f);
+				free(zeroes);
+			}
+		}
+	}
 
 	if (ferror(f))
 		die("Error writing device tree blob: %s\n", strerror(errno));

@@ -504,6 +529,14 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)
 
 	emit_label(f, symprefix, "blob_end");
 
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		fprintf(f, "\t.space\t%d - (%s_blob_end - %s_blob_start), 0\n", 
+			minsize, symprefix, symprefix);
+	}
+
 	data_free(strbuf);
 }

Signature · 1 line

1.4.4.4

David Gibson

2007-04-18 03:05 UTC #1 lore raw

Quoted 10 lines · Jerry Van Baren

If the user requests extra space, pad out the blob (previously the unused
data was undefined).
Signed-off-by Gerald Van Baren <[email protected]>
---
Hi Jon, David, Milton,
This improves the -S option to pad out the blob with zeros when the user asks for extra space.

Comment below

Quoted 58 lines

flattree.c +16 -1

index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,

@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 
+	size = 0;
+
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
 			vi = &version_table[i];

@@ -384,10 +388,13 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 		 boot_cpuid_phys);
 
 	fwrite(&bph, vi->hdr_size, 1, f);
+	size += vi->hdr_size;
 
 	/* Align the reserve map to an 8 byte boundary */
-	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++)
+	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++) {
 		fputc(0, f);
+		size += 1;
+	}
 
 	/*
 	 * Reserve map entries.

@@ -396,9 +403,27 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 	 */
 	fwrite(reservebuf.val, reservebuf.len, 1, f);
 	fwrite(&termre, sizeof(termre), 1, f);
+	size += reservebuf.len + sizeof(termre);
 
 	fwrite(dtbuf.val, dtbuf.len, 1, f);
 	fwrite(strbuf.val, strbuf.len, 1, f);
+	size += dtbuf.len + strbuf.len;
+
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		int padlen = minsize - size;

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version, · line 426

Quoted 10 lines

flattree.c +10 -0

+
+		if (padlen > 0) {
+			char *zeroes = calloc(padlen, 1);
+
+			if (zeroes != NULL) {
+				fwrite(zeroes, padlen, 1, f);
+				free(zeroes);
+			}
+		}
+	}

Hrm, rather than all this explicit calloc() mangling, I'd prefer we use our existing struct data stuff. Use data_append_zeroes() on empty_data to generate your buffer of padding.

Quoted 17 lines

flattree.c +8 -0

 	if (ferror(f))
 		die("Error writing device tree blob: %s\n", strerror(errno));

@@ -504,6 +529,14 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)
 
 	emit_label(f, symprefix, "blob_end");
 
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		fprintf(f, "\t.space\t%d - (%s_blob_end - %s_blob_start), 0\n", 
+			minsize, symprefix, symprefix);
+	}
+
 	data_free(strbuf);
 }

Signature · 4 lines

David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

David Gibson

2007-04-18 06:45 UTC #2 lore raw

Quoted 36 lines · David Gibson

Quoted 10 lines · Jerry Van Baren
If the user requests extra space, pad out the blob (previously the unused
data was undefined).
Signed-off-by Gerald Van Baren <[email protected]>
---
Hi Jon, David, Milton,
This improves the -S option to pad out the blob with zeros when the user asks for extra space.
Comment below
Quoted 23 lines
flattree.c +2 -0
index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,
@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 

Quoted 2 lines

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version, · line 370
Quoted 2 lines
flattree.c +2 -0
+	size = 0;
+

Oh, also, you shouldn't need this new variable to track the size. Just use whatever expression we used to use for totalsize before we forced it out to the minimum size.

Signature · 4 lines

David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Jerry Van Baren

2007-04-18 12:27 UTC #3 lore raw

Quoted 36 lines · David Gibson

Quoted 35 lines · David Gibson
Quoted 10 lines · Jerry Van Baren
If the user requests extra space, pad out the blob (previously the unused
data was undefined).
Signed-off-by Gerald Van Baren <[email protected]>
---
Hi Jon, David, Milton,
This improves the -S option to pad out the blob with zeros when the user asks for extra space.
Comment below
Quoted 23 lines
flattree.c +2 -0
index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,
@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 

Quoted 6 lines

Quoted 2 lines
flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version, · line 370
Quoted 2 lines
flattree.c +2 -0
+	size = 0;
+
Oh, also, you shouldn't need this new variable to track the size. Just use whatever expression we used to use for totalsize before we forced it out to the minimum size.

It was simpler that way. Much simpler. With the calculated version, I had to know the header size and re-calculate all the alignment gaps. Ugly and nasty (respectively and collectively).

Best regards,
gvb

David Gibson

2007-04-19 00:18 UTC #4 lore raw

Quoted 44 lines · Jerry Van Baren

Quoted 36 lines · David Gibson
Quoted 35 lines · David Gibson
Quoted 10 lines · Jerry Van Baren
If the user requests extra space, pad out the blob (previously the unused
data was undefined).
Signed-off-by Gerald Van Baren <[email protected]>
---
Hi Jon, David, Milton,
This improves the -S option to pad out the blob with zeros when the user asks for extra space.
Comment below
Quoted 23 lines
flattree.c +2 -0
index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,
@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 
Quoted 6 lines
Quoted 2 lines
flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version, · line 370
Quoted 2 lines
flattree.c +2 -0
+	size = 0;
+
Oh, also, you shouldn't need this new variable to track the size. Just use whatever expression we used to use for totalsize before we forced it out to the minimum size.

Quoted 3 lines

It was simpler that way. Much simpler. With the calculated version, I had to know the header size and re-calculate all the alignment gaps. Ugly and nasty (respectively and collectively).

Hrm.. but we already did all that calculation in make_bph().

Here's a revised idea: with -S, instead of overriding the totalsize header field directly in make_bph(), instead revert that function to always computing the "natural minimum" size of the blob. Then do the -S handling in dt_to_blob() instead: compute the number of padding bytes necessary from make_bph()'s totalsize value, then overwrite the totalsize field with the expanded value.

Signature · 4 lines

David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Jerry Van Baren

2007-04-18 12:19 UTC #5 lore raw

In reply to David Gibson

Quoted 92 lines · David Gibson

Quoted 10 lines · Jerry Van Baren

If the user requests extra space, pad out the blob (previously the unused
data was undefined).
Signed-off-by Gerald Van Baren <[email protected]>
---
Hi Jon, David, Milton,
This improves the -S option to pad out the blob with zeros when the user asks for extra space.

Comment below

Quoted 58 lines

flattree.c +16 -1

index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,

@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 
+	size = 0;
+
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
 			vi = &version_table[i];

@@ -384,10 +388,13 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 		 boot_cpuid_phys);
 
 	fwrite(&bph, vi->hdr_size, 1, f);
+	size += vi->hdr_size;
 
 	/* Align the reserve map to an 8 byte boundary */
-	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++)
+	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++) {
 		fputc(0, f);
+		size += 1;
+	}
 
 	/*
 	 * Reserve map entries.

@@ -396,9 +403,27 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 	 */
 	fwrite(reservebuf.val, reservebuf.len, 1, f);
 	fwrite(&termre, sizeof(termre), 1, f);
+	size += reservebuf.len + sizeof(termre);
 
 	fwrite(dtbuf.val, dtbuf.len, 1, f);
 	fwrite(strbuf.val, strbuf.len, 1, f);
+	size += dtbuf.len + strbuf.len;
+
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		int padlen = minsize - size;

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version, · line 426

Quoted 10 lines

flattree.c +10 -0

+
+		if (padlen > 0) {
+			char *zeroes = calloc(padlen, 1);
+
+			if (zeroes != NULL) {
+				fwrite(zeroes, padlen, 1, f);
+				free(zeroes);
+			}
+		}
+	}

Hrm, rather than all this explicit calloc() mangling, I'd prefer we use our existing struct data stuff. Use data_append_zeroes() on empty_data to generate your buffer of padding.

Quoted 6 lines

flattree.c +0 -0

 	if (ferror(f))
 		die("Error writing device tree blob: %s\n", strerror(errno));

@@ -504,6 +529,14 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)
 
 	emit_label(f, symprefix, "blob_end");

Quoted 10 lines

flattree.c void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys) · line 541
Quoted 10 lines
flattree.c +8 -0
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		fprintf(f, "\t.space\t%d - (%s_blob_end - %s_blob_start), 0\n", 
+			minsize, symprefix, symprefix);
+	}
+
 	data_free(strbuf);
 }

Hi David,

I started down the data_append_zeroes() path and had to back up and take the branch less traveled ;-). The problem is that data_append_zeros() appends the data to a "struct data" and I don't want to add zeros internally to any of the existing pieces parts since the extra space is external, not internal to the existing parts.

I suppose I could create a new "struct data zeroes" and use data_append_zeroes() to create zeros in it, write that out, and then free it, but I fail to see how that is better than simply calloc()ing a bunch of zeros and writing _that_ out.

Best regards,
gvb

P.S. David: I cannot send email to your dropbear.id.au address from home, one of our ISPs apparently has the other blackballed. I mention this just so you know I'm not intentionally cutting you out of the To: list.

David Gibson

2007-04-19 01:42 UTC #6 lore raw

Quoted 112 lines · Jerry Van Baren

Quoted 92 lines · David Gibson

Quoted 10 lines · Jerry Van Baren

If the user requests extra space, pad out the blob (previously the unused
data was undefined).
Signed-off-by Gerald Van Baren <[email protected]>
---
Hi Jon, David, Milton,
This improves the -S option to pad out the blob with zeros when the user asks for extra space.

Comment below

Quoted 58 lines

flattree.c +16 -1

index 151d16e..d2ee0dc 100644
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,

@@ -358,12 +359,15 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
+	int size;
 	struct data dtbuf = empty_data;
 	struct data strbuf = empty_data;
 	struct data reservebuf;
 	struct boot_param_header bph;
 	struct reserve_entry termre = {.address = 0, .size = 0};
 
+	size = 0;
+
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
 			vi = &version_table[i];

@@ -384,10 +388,13 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 		 boot_cpuid_phys);
 
 	fwrite(&bph, vi->hdr_size, 1, f);
+	size += vi->hdr_size;
 
 	/* Align the reserve map to an 8 byte boundary */
-	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++)
+	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++) {
 		fputc(0, f);
+		size += 1;
+	}
 
 	/*
 	 * Reserve map entries.

@@ -396,9 +403,27 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 	 */
 	fwrite(reservebuf.val, reservebuf.len, 1, f);
 	fwrite(&termre, sizeof(termre), 1, f);
+	size += reservebuf.len + sizeof(termre);
 
 	fwrite(dtbuf.val, dtbuf.len, 1, f);
 	fwrite(strbuf.val, strbuf.len, 1, f);
+	size += dtbuf.len + strbuf.len;
+
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		int padlen = minsize - size;

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version, · line 426

Quoted 10 lines

flattree.c +10 -0

+
+		if (padlen > 0) {
+			char *zeroes = calloc(padlen, 1);
+
+			if (zeroes != NULL) {
+				fwrite(zeroes, padlen, 1, f);
+				free(zeroes);
+			}
+		}
+	}

Hrm, rather than all this explicit calloc() mangling, I'd prefer we use our existing struct data stuff. Use data_append_zeroes() on empty_data to generate your buffer of padding.

Quoted 6 lines

flattree.c +0 -0

 	if (ferror(f))
 		die("Error writing device tree blob: %s\n", strerror(errno));

@@ -504,6 +529,14 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)
 
 	emit_label(f, symprefix, "blob_end");

Quoted 10 lines

flattree.c void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys) · line 541
Quoted 10 lines
flattree.c +8 -0
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		fprintf(f, "\t.space\t%d - (%s_blob_end - %s_blob_start), 0\n", 
+			minsize, symprefix, symprefix);
+	}
+
 	data_free(strbuf);
 }

Hi David,

Quoted 4 lines

I suppose I could create a new "struct data zeroes" and use data_append_zeroes() to create zeros in it, write that out, and then free it, but I fail to see how that is better than simply calloc()ing a bunch of zeros and writing _that_ out.

Hrm. I suppose. Though I do notice that your code will silently omit the padding if the calloc() fails.

Quoted 3 lines

P.S. David: I cannot send email to your dropbear.id.au address from home, one of our ISPs apparently has the other blackballed. I mention this just so you know I'm not intentionally cutting you out of the To: list.

Eck. That's a pain.

Signature · 4 lines

David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson