[M] Change in coreboot[master]: lib/coreboot_table: Simplify API to set up lb_serial

[Arthur Heymans (Code Review)]

Attention is currently required from: Hung-Te Lin, Julius Werner, ron minnich.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68768 )

Change subject: lib/coreboot_table: Simplify API to set up lb_serial
......................................................................

Patch Set 7:

(1 comment)

File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/68768/comment/fa1740fd_fb76ff15 
PS7, Line 102: struct lb_serial new_serial = get_lb_serial();
             : 	/* Use the base address as a proxy for an invalid struct */
             : 	if (new_serial.baseaddr == 0)
             : 		return;
             : 
             : 	serial = (struct lb_serial *)lb_new_record(header);
             : 	memcpy(serial, &new_serial, sizeof(*serial));
             : 	serial->tag = LB_TAG_SERIAL;
             : 	serial->size = sizeof(*serial);
             : 	if (serial->type == LB_SERIAL_TYPE_IO_MAPPED)
             : 		lb_add_console(LB_TAG_CONSOLE_SERIAL8250, header);
             : 	else
             : 		lb_add_console(LB_TAG_CONSOLE_SERIAL8250MEM, header);
> It's true we can return a struct, but the implementation above is
> using a hint baseaddr=0 implies invalid
> assuming the get_lb_serial will only fill partial data
> overwriting the size
>
> If we're going to change the APIs, I'd rather redefine it as:
>   int get_lb_serial(struct lb_serial *serial) {
>     serial->type = ...
>     return 0;
>   }
>
> And then you can do here:
>
> struct lb_serial new_serial = {0};
>   new_serial.tag = LB_TAG_SERIAL;
>   new_serial.size = sizeof(new_serial);
>   if (!get_lb_serial(&new_serial))
>     return;
>   // allocate the new record, copy serial etc

Nice suggestion.