[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add private port structure, and move iconv descriptors there
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] Add private port structure, and move iconv descriptors there |
Date: |
Wed, 27 Mar 2013 21:28:28 +0100 |
User-agent: |
Gnus/5.130005 (Ma Gnus v0.5) Emacs/24.2 (gnu/linux) |
Hi!
Mark H Weaver <address@hidden> 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’.
- Re: Adding new information to scm_t_port (was Re: always O_BINARY?), Ludovic Courtès, 2013/03/01
- Re: Adding new information to scm_t_port (was Re: always O_BINARY?), Mark H Weaver, 2013/03/05
- [PATCH] Add private port structure, and move iconv descriptors there, Mark H Weaver, 2013/03/27
- Re: [PATCH] Add private port structure, and move iconv descriptors there,
Ludovic Courtès <=
- Re: [PATCH] Add private port structure, and move iconv descriptors there, Andy Wingo, 2013/03/27
- Re: [PATCH] Add private port structure, and move iconv descriptors there, Mark H Weaver, 2013/03/27
- [PATCH] Add internal-only port structure; move iconv descriptors there, Mark H Weaver, 2013/03/31
- Re: [PATCH] Add internal-only port structure; move iconv descriptors there, Ludovic Courtès, 2013/03/31
- Re: [PATCH] Add internal-only port structure; move iconv descriptors there, Mark H Weaver, 2013/03/31
- Re: [PATCH] Add internal-only port structure; move iconv descriptors there, Ludovic Courtès, 2013/03/31
- Re: [PATCH] Add private port structure, and move iconv descriptors there, Mark H Weaver, 2013/03/31
- Re: [PATCH] Add private port structure, and move iconv descriptors there, Ludovic Courtès, 2013/03/31