Re: [PATCH v2 17/18] mini-os: use function vectors instead of switch for file operations

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, le mar. 11 janv. 2022 15:58:16 +0100, a ecrit:
> @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes)
>      return -1;
>  }
>  
> +off_t lseek_default(int fd, off_t offset, int whence)
> +{
> +    switch ( whence )
> +    {

Is there a reason for making this one a separate function, unlike others
for which you kept the code in the main function?

Apart from that, this looks good to me.

> +    case SEEK_SET:
> +        files[fd].offset = offset;
> +        break;
> +
> +    case SEEK_CUR:
> +        files[fd].offset += offset;
> +        break;
> +
> +    case SEEK_END:
> +    {
> +        struct stat st;
> +        int ret;
> +
> +        ret = fstat(fd, &st);
> +        if ( ret )
> +            return -1;
> +        files[fd].offset = st.st_size + offset;
> +        break;
> +    }
> +
> +    default:
> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    return files[fd].offset;
> +}
> +
>  off_t lseek(int fd, off_t offset, int whence)
>  {
> +    struct file_ops *ops = get_file_ops(files[fd].type);
> +
> +    if ( ops->lseek )
> +        return ops->lseek(fd, offset, whence);
> +
>      switch(files[fd].type) {
>  #ifdef CONFIG_BLKFRONT
>         case FTYPE_BLK:
> @@ -393,28 +473,7 @@ off_t lseek(int fd, off_t offset, int whence)
>            return (off_t) -1;
>      }
>  
> -    switch (whence) {
> -       case SEEK_SET:
> -          files[fd].offset = offset;
> -          break;
> -       case SEEK_CUR:
> -          files[fd].offset += offset;
> -          break;
> -       case SEEK_END:
> -          {
> -             struct stat st;
> -             int ret;
> -             ret = fstat(fd, &st);
> -             if (ret)
> -                return -1;
> -             files[fd].offset = st.st_size + offset;
> -             break;
> -          }
> -       default:
> -          errno = EINVAL;
> -          return -1;
> -    }
> -    return files[fd].offset;
> +    return lseek_default(fd, offset, whence);
>  }
>  
>  int fsync(int fd) {