Re: [PATCH] Add private port structure, and move iconv descriptors there

[ludo@gnu.org (Ludovic Courtès)]

Hi!

Mark H Weaver <mhw@netris.org> skribis:

> Here's a patch to add a private port structure.  I moved both input_cd
> and output_cd there.  I plan to create more patches on top of this very
> soon (BOM handling, more efficient per-port read options, maybe better
> EOF handling), so a prompt review would be very helpful.

Looks good to me, and definitely an improvement.

Nitpicking:

> +struct scm_t_port_private;

Please add a short comment above.

> +  struct scm_t_port_private *internal;
> +  void *reserved_for_future_use;
>  } scm_t_port;

Likewise.

> +enum scm_t_port_encoding_type {

Likewise.

Also, brace on the next line.

I’d be tempted to remove ‘_t’ from the name since it’s a tag.

> +struct scm_t_port_private

Likewise.

> +  enum scm_t_port_encoding_type encoding_type;
> +  void *input_cd;
> +  void *output_cd;
> +};

Ideally a comment saying what the fields represent.

Thanks!

Ludo’.