🤪
次の行を読み込む関数を作成する
課題の内容
42の規則上課題のPDFを掲載できないのですが、ざっくりいうと渡したファイルを1行ずつ読み込んでくれる関数です。
forやswitchなどがコード規約で禁止されているため、一部モダンじゃない書き方をしています。
それ以外で「ここもっとスッキリかけそう」と言う箇所があればお知らせください。
また、使用できる標準関数はreadとmallocおよびfreeのみです。
ft_で始まる関数は標準関数を自作したもので、挙動は同じになっているはずです、
ソースコード
get_next_line.h
#ifndef GET_NEXT_LINE_H
# define GET_NEXT_LINE_H
# define SUCCESS 1
# define END_OF_FILE 0
# define READ_ERROR -1
# include <unistd.h>
# include <stdlib.h>
int get_next_line(int fd, char **line);
size_t ft_strlen(const char *s);
char *ft_strchr(char *s, int c);
void *ft_memmove(void *dst, const void *src, size_t len);
char *ft_substr(char const *s, unsigned int start, size_t len);
#endif
get_next_line.c
#include "get_next_line.h"
static char *free_return(char *save)
{
free(save);
save = NULL;
return (NULL);
}
static int count_till_endl(char *s)
{
int i;
i = 0;
if (!s)
return (0);
while (s[i] && s[i] != '\n')
i++;
return (i);
}
static char *gnl_strjoin(char const *s1, char const *s2)
{
size_t len1;
size_t len2;
size_t total_len;
char *ret;
if (!s1 && !s2)
return (NULL);
len1 = ft_strlen((char *)s1);
len2 = ft_strlen((char *)s2);
total_len = len1 + len2;
if (!(ret = (char *)malloc((total_len + 1) * sizeof(char))))
return (NULL);
ft_memmove(ret, s1, len1);
ft_memmove(ret + len1, s2, len2);
ret[total_len] = '\0';
free((char *)s1);
return (ret);
}
static char *get_save(char *save)
{
char *ret;
int i;
int j;
i = 0;
j = 0;
if (!save)
return (NULL);
while (save[i] != '\n')
{
if (!save[i])
return (free_return(save));
i++;
}
if (!(ret = malloc(sizeof(char) * ((ft_strlen(save) - i) + 1))))
return (NULL);
i++;
while (save[i])
ret[j++] = save[i++];
ret[j] = '\0';
free(save);
return (ret);
}
int get_next_line(int fd, char **line)
{
int read_res;
char *buf;
static char *save;
read_res = SUCCESS;
if (fd < 0 || !line || BUFFER_SIZE < 1 ||
!(buf = (char *)malloc((size_t)BUFFER_SIZE + 1)))
return (READ_ERROR);
while (!(ft_strchr(save, '\n')) && read_res != 0)
{
if ((read_res = read(fd, buf, BUFFER_SIZE)) == READ_ERROR)
{
free(buf);
return (READ_ERROR);
}
buf[read_res] = '\0';
save = gnl_strjoin(save, buf);
}
free(buf);
*line = ft_substr(save, 0, count_till_endl(save));
save = get_save(save);
return ((read_res == END_OF_FILE) ? END_OF_FILE : SUCCESS);
}
main.c
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include "get_next_line.h"
/*main.cは提出しないため、printfとか使ってもOKです*/
int main(int argc, char **argv)
{
int fd;
char *line;
if (argc == 1 || argc == 2)
{
fd = (argc == 1) ? 0 : open(argv[1], P_RDONLY);
while (get_next_line(fd, &line) == 1)
{
printf("%s\n", line);
free(line);
}
close(fd);
}
else
return (2);
system("leaks a.out");
return (0);
}
Discussion
strchrの第1引数の型はconst char*の筈ですがft_strchrのそれがchar*となっているのはミスだと思います。あと、Cの標準関数に
substrは存在しないと思いますが記事のコードではft_substrというのが使用されており「ft_で始まる関数は標準関数を自作したもの」という説明とは矛盾しています。開始位置と長さを表す引数型で
unsigned intとsize_tが混用されているのは良いデザインではないですね。get_next_line.hの中で、関数プロトタイプに使用してる型を定義しているヘッダをインクルードするのであれば分かりますが、そうでないヘッダ
はインクルードすべきではないですね。
size_tを使いたいということであればで十分だと思います。
get_next_line.hでの関数プロトタイプはget_next_line.cで定義された関数にとどめるべきであり、それ以外の関数
は別のところで関数プロトタイプを定義するべきと思います。
const char *とchar const *が混在してますがひとつのプログラムであればどちらかに統一した方が良いですね。save = NULL;は無意味なことやられてる印象ですが、解放したメモリブロックを格納したポインタ変数にNULLを格納したいということであればとでもされたら良いかもしれないですね。
この関数の中の処理では
s1とs2はどちらも有効な文字列を指してる想定で動作するものと思うので、の
&&は||が正しいんじゃないでしょうか?ft_strlenの引数型はconst char*なので、↑のconst外しはまったく意味がないですね。
なんでか
s1をfreeしてますが文字列の結合とは関係ないのでこの関数の外で行うべきですね。文字列の'\0'か'\n'を検索したいということであればすでに用意済みの
ft_strchrを使うべきではないですか?文字列の添え字が
intですが他方ではsize_tやunsigned intを使用されておりポリシーを感じません。仮引数
sの指してる先は読んでるだけなので型はconst char*にすべきですね。まず
get_saveという関数名が意味が分かりません。'\n'を検索されたいのであれば
ft_strchrを使うべきでは?文字列の添え字がintで宜しくないのは
count_till_endlと同様です。文字列コピーは
strcpy等使用するべきですね。ft_strlenやft_strchrがあってft_strcpyがない理屈はないと思います。定義済の
free_returnを使われてないのは一貫性に欠けますね。saveはstatic変数であり初期化子がないためNULLで初期化されますがそれを
ft_strchrに渡しています。標準関数のstrchrは第1引数にNULL渡すことはできませんがそれを平気でやっていることから察するにft_strchrは記事の冒頭に説明されているとはなっておらず、
count_till_endlの実装のように引数の
NULL判定を行いNULLか何かを返すよう実装されていると思います。標準関数の動作としては未定義動作となるためそのような実装も許されないわけではありませんが、バグの発見を遅らせ問題を先送りにする行為なので褒められたものではありません。また、
NULLが渡されても動作することを前提に呼び出しを行ってしまうと標準関数との互換性も失われてしまうこととなります。この関数は
NULLを長さ0の文字列として扱う作りになっていますが、大変筋が悪いです。gnl_strjoinやget_saveが中で呼んでいるmallocがメモリ取得に失敗してNULLを返した場合それはエラーとして扱われるべきですがこの関数ではNULLは長さ0の文字列として扱っているためエラーの検出がされずsaveの指していた文字列は捨てられメモリリークを起こした上で先へ進んでしまいます。また、
saveにメモリが割り当てられた状態でmallocやreadがエラーを返した場合、saveを開放する処理がないためメモリリークとなります。あと
EOFに達した場合、*lineに長さ0の文字列を書き込むため、mainからのget_next_lineの使い方がこの通りではメモリリークが発生すると思います。
#
P_RDONLYはO_RDONLYのtypoでしょうか?get_next_lineはファイル記述子を仮引数として受け取る作りになっていますが、動作をひとつのstatic変数に依存しているため複数のファイル記述子に対応できない構造になっているのは設計バグとも思います。自己レス:
gcc 15.2に
-fsanitize=addressを指定してコンパイル、実行し確認してみました。と
を追加し、
mainのprintfの後にはfflush(stdout);を追加しています。他、足りない関数は
に従い以下の実装を加えています。